Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

kvdb-rocksdb: remove buffered operations when committing transaction #7950

Merged
merged 1 commit into from
Feb 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions util/kvdb-rocksdb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,9 @@ impl Database {
let batch = WriteBatch::new();
let ops = tr.ops;
for op in ops {
// remove any buffered operation for this key
self.overlay.write()[Self::to_overlay_column(op.col())].remove(op.key());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I should take a read lock first to check if there's any entry, instead of always locking for write.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No sure either; is there any reliable way to just benchmark this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe, we always lock for write


match op {
DBOp::Insert { col, key, value } => {
col.map_or_else(|| batch.put(&key, &value), |c| batch.put_cf(cfs[c as usize], &key, &value))?
Expand Down Expand Up @@ -857,4 +860,21 @@ mod tests {
assert_eq!(db.num_columns(), 0);
}
}

#[test]
fn write_clears_buffered_ops() {
let tempdir = TempDir::new("").unwrap();
let config = DatabaseConfig::default();
let db = Database::open(&config, tempdir.path().to_str().unwrap()).unwrap();

let mut batch = db.transaction();
batch.put(None, b"foo", b"bar");
db.write_buffered(batch);

let mut batch = db.transaction();
batch.put(None, b"foo", b"baz");
db.write(batch).unwrap();

assert_eq!(db.get(None, b"foo").unwrap().unwrap().as_ref(), b"baz");
}
}
20 changes: 20 additions & 0 deletions util/kvdb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,26 @@ pub enum DBOp {
}
}

impl DBOp {
/// Returns the key associated with this operation.
pub fn key(&self) -> &[u8] {
match *self {
DBOp::Insert { ref key, .. } => key,
DBOp::InsertCompressed { ref key, .. } => key,
DBOp::Delete { ref key, .. } => key,
}
}

/// Returns the column associated with this operation.
pub fn col(&self) -> Option<u32> {
match *self {
DBOp::Insert { col, .. } => col,
DBOp::InsertCompressed { col, .. } => col,
DBOp::Delete { col, .. } => col,
}
}
}

impl DBTransaction {
/// Create new transaction.
pub fn new() -> DBTransaction {
Expand Down