Skip to content

Commit

Permalink
Remove reward from PostCommitWork
Browse files Browse the repository at this point in the history
It's not needed, we can do it safely without embedding T in there.
  • Loading branch information
anacrolix committed Jul 10, 2024
1 parent b9a432e commit 4ca4296
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 18 deletions.
9 changes: 5 additions & 4 deletions src/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ impl Handle {
// Maybe it's okay just to commit anyway, since we have a deferred transaction and sqlite
// might know nothing has changed.
if deleted.is_some() {
tx.commit(())?.complete();
tx.commit()?.complete();
}
Ok(deleted)
}
Expand All @@ -319,7 +319,8 @@ impl Handle {
pub fn rename_item(&mut self, from: &[u8], to: &[u8]) -> PubResult<Timestamp> {
let mut tx = self.start_immediate_transaction()?;
let last_used = tx.rename_item(from, to)?;
Ok(tx.commit(last_used)?.complete())
tx.commit()?.complete();
Ok(last_used)
}

/// Walks the underlying files in the possum directory.
Expand Down Expand Up @@ -451,7 +452,7 @@ impl Handle {
to_vec.extend_from_slice(item.key.strip_prefix(from).unwrap());
tx.rename_item(&item.key, &to_vec)?;
}
tx.commit(())?.complete();
tx.commit()?.complete();
Ok(())
}

Expand All @@ -460,7 +461,7 @@ impl Handle {
for item in tx.list_items(prefix)? {
tx.delete_key(&item.key)?;
}
tx.commit(())?.complete();
tx.commit()?.complete();
Ok(())
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,12 +344,11 @@ impl<'handle> BatchWriter<'handle> {
transaction.rename_value(&vr.value, vr.new_key)?;
}
// TODO: On error here, rewind the exclusive to undo any writes that just occurred.
let work = transaction
.commit(write_commit_res)
.context("commit transaction")?;
let work = transaction.commit().context("commit transaction")?;

self.flush_exclusive_files();
Ok(work.complete())
work.complete();
Ok(write_commit_res)
}

/// Flush Writer's exclusive files and return them to the Handle pool.
Expand Down
4 changes: 2 additions & 2 deletions src/ownedtx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ impl DerefMut for OwnedTx<'_> {

impl<'a> OwnedTx<'a> {
// Except for this move dependent dance it shouldn't be necessary to wrap the OwnedCell.
pub fn commit<T>(self, reward: T) -> Result<PostCommitWork<'a, T>> {
self.cell.move_dependent(|tx| tx.commit(reward))
pub fn commit(self) -> Result<PostCommitWork<'a>> {
self.cell.move_dependent(|tx| tx.commit())
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl<'a> Reader<'a> {
pub fn begin(self) -> Result<Snapshot> {
let file_clones = self.clone_files().context("cloning files")?;
self.owned_tx
.commit(())
.commit()
.context("committing transaction")?
.complete();
Ok(Snapshot { file_clones })
Expand Down
11 changes: 4 additions & 7 deletions src/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ use super::*;

/// This is more work to be done after the Handle conn mutex is released.
#[must_use]
pub(crate) struct PostCommitWork<'h, T> {
pub(crate) struct PostCommitWork<'h> {
handle: &'h Handle,
deleted_values: Vec<NonzeroValueLocation>,
altered_files: HashSet<FileId>,
reward: T,
}

/// Exposes a rusqlite Transaction to implement ReadTransaction.
Expand Down Expand Up @@ -146,8 +145,8 @@ fn list_items_inner(
.map_err(Into::into)
}

impl<'h, T> PostCommitWork<'h, T> {
pub fn complete(self) -> T {
impl<'h> PostCommitWork<'h> {
pub fn complete(self) {
// This has to happen after exclusive files are flushed or there's a tendency for hole
// punches to not persist. It doesn't fix the problem, but it significantly reduces it.
if !self.handle.instance_limits.disable_hole_punching {
Expand All @@ -157,7 +156,6 @@ impl<'h, T> PostCommitWork<'h, T> {
for file_id in self.altered_files {
self.handle.clones.lock().unwrap().remove(&file_id);
}
self.reward
}
}

Expand Down Expand Up @@ -190,14 +188,13 @@ impl<'h> Transaction<'h> {
}
}

pub(crate) fn commit<T>(mut self, reward: T) -> Result<PostCommitWork<'h, T>> {
pub(crate) fn commit(mut self) -> Result<PostCommitWork<'h>> {
self.apply_limits()?;
self.tx.commit()?;
Ok(PostCommitWork {
handle: self.handle,
deleted_values: self.deleted_values,
altered_files: self.altered_files,
reward,
})
}

Expand Down

0 comments on commit 4ca4296

Please sign in to comment.