Skip to content

Commit

Permalink
fix: allow get_history api to read from write set
Browse files Browse the repository at this point in the history
  • Loading branch information
arriqaaq committed Jan 13, 2025
1 parent 916f7fa commit 35ff93d
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 17 deletions.
113 changes: 112 additions & 1 deletion src/snapshot.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::ops::RangeBounds;
use vart::{art::QueryType, art::Tree, iter::Iter, VariableSizeKey};
use vart::{art::Tree, iter::Iter, VariableSizeKey};

use crate::error::{Error, Result};
use crate::indexer::IndexValue;
Expand Down Expand Up @@ -227,4 +227,115 @@ mod tests {
);
}
}

#[tokio::test]
async fn test_get_history_with_write_set() {
let temp_dir = create_temp_directory();
let mut opts = Options::new();
opts.dir = temp_dir.path().to_path_buf();
let store = Store::new(opts).expect("Failed to create store");
let key = b"test_key";

// Test 1: Only write set value
{
let mut txn = store.begin().expect("Failed to begin transaction");
txn.set(key, b"write_set_value")
.expect("Failed to set value");

let history = txn.get_history(key).expect("Failed to get history");
assert_eq!(history.len(), 1, "Should have one entry from write set");
assert_eq!(history[0].0, b"write_set_value");

// Verify timestamp is recent (the latest entry version is 0)
assert_eq!(history[0].1, 0);
}

// Test 2: Value in both write set and snapshot
{
// First commit a value to create snapshot
let mut txn = store.begin().expect("Failed to begin transaction");
txn.set(key, b"snapshot_value")
.expect("Failed to set value");
txn.commit().await.expect("Failed to commit");

// Now create new transaction and set write set value
let mut txn = store.begin().expect("Failed to begin transaction");
txn.set(key, b"write_set_value")
.expect("Failed to set value");

let history = txn.get_history(key).expect("Failed to get history");
assert_eq!(
history.len(),
2,
"Should have entries from both write set and snapshot"
);

// Write set value should be more recent (first in history)
assert_eq!(history[0].0, b"write_set_value");
assert_eq!(history[1].0, b"snapshot_value");
}

// Test 3: Multiple snapshot versions plus write set
{
// Commit multiple versions
let mut txn = store.begin().expect("Failed to begin transaction");
txn.set(key, b"snapshot_value1")
.expect("Failed to set value");
txn.commit().await.expect("Failed to commit");

let mut txn = store.begin().expect("Failed to begin transaction");
txn.set(key, b"snapshot_value2")
.expect("Failed to set value");
txn.commit().await.expect("Failed to commit");

// Now add write set value
let mut txn = store.begin().expect("Failed to begin transaction");
txn.set(key, b"write_set_value")
.expect("Failed to set value");

let history = txn.get_history(key).expect("Failed to get history");
assert_eq!(
history.len(),
4,
"Should have write set + 3 snapshot entries"
);

assert_eq!(
history[0].0, b"write_set_value",
"Most recent should be write set value"
);
assert_eq!(
history[1].0, b"snapshot_value",
"First should be oldest snapshot"
);
assert_eq!(
history[2].0, b"snapshot_value1",
"Second should be latest snapshot"
);
assert_eq!(
history[3].0, b"snapshot_value2",
"Third should be latest snapshot"
);
}

// Test 4: Deleted value in write set
{
let mut txn = store.begin().expect("Failed to begin transaction");
txn.set(key, b"initial_value").expect("Failed to set value");
txn.commit().await.expect("Failed to commit");

let mut txn = store.begin().expect("Failed to begin transaction");
txn.delete(key).expect("Failed to delete key");

let history = txn.get_history(key).expect("Failed to get history");
assert_eq!(history.len(), 4, "Should only have snapshot values");
assert_eq!(history[3].0, b"initial_value");
}

// Test 5: Empty key
{
let txn = store.begin().expect("Failed to begin transaction");
assert!(txn.get_history(b"").is_err());
}
}
}
28 changes: 12 additions & 16 deletions src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::sync::Arc;

use ahash::{HashSet, HashSetExt};
use bytes::Bytes;
use vart::{art::QueryType, VariableSizeKey};
use vart::VariableSizeKey;

use crate::entry::Entry;
use crate::error::{Error, Result};
Expand Down Expand Up @@ -608,18 +608,6 @@ impl Transaction {
/// Implement Versioned APIs for read-only transactions.
/// These APIs do not take part in conflict detection.
impl Transaction {
fn ensure_read_only_transaction(&self) -> Result<()> {
// If the transaction is closed, return an error.
if self.closed {
return Err(Error::TransactionClosed);
}
// Do not allow versioned reads if it is not a read-only transaction
if !self.mode.is_read_only() {
return Err(Error::TransactionMustBeReadOnly);
}
Ok(())
}

/// Returns the value associated with the key at the given timestamp.
pub fn get_at_ts(&self, key: &[u8], ts: u64) -> Result<Option<Vec<u8>>> {
// If the key is empty, return an error.
Expand Down Expand Up @@ -658,15 +646,20 @@ impl Transaction {

/// Returns all the versioned values and timestamps associated with the key.
pub fn get_history(&self, key: &[u8]) -> Result<Vec<(Vec<u8>, u64)>> {
self.ensure_read_only_transaction()?;

// If the key is empty, return an error.
if key.is_empty() {
return Err(Error::EmptyKey);
}

let mut results = Vec::new();

// Check write set first
if let Some(write_val) = self.get_in_write_set(key) {
if let Some(val) = write_val.0 {
results.push((val.to_vec(), write_val.1));
}
}

// Attempt to get the value for the key from the snapshot.
match self
.snapshot
Expand Down Expand Up @@ -735,7 +728,10 @@ impl Transaction {
KeyScanIterator::new(&self.write_set, snap_iter, &range, None)
}

/// Scans a range of keys and returns a vector of tuples containing the key, value, timestamp, and deletion status for each key.
/// Scans a range of keys and returns a vector of tuples containing the key, value, timestamp,
/// and deletion status for each key.
///
/// This does not take part in conflict detection. And does not return in-flight writes.
pub fn scan_all_versions<'a, R>(
&'a self,
range: R,
Expand Down

0 comments on commit 35ff93d

Please sign in to comment.