From 334551aaccba010cade2ae6f90173c36d48a8a72 Mon Sep 17 00:00:00 2001 From: Farhan Date: Mon, 18 Mar 2024 16:01:12 +0530 Subject: [PATCH] chore: remove snapshot counters --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/art.rs | 100 ++++++------------------------------------------ src/iter.rs | 7 +--- src/lib.rs | 6 --- src/snapshot.rs | 65 ++----------------------------- 6 files changed, 18 insertions(+), 164 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a80638d..5a7bb1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -577,7 +577,7 @@ checksum = "301abaae475aa91687eb82514b328ab47a211a533026cb25fc3e519b86adfc3c" [[package]] name = "vart" -version = "0.1.1" +version = "0.1.2" dependencies = [ "criterion", "hashbrown", diff --git a/Cargo.toml b/Cargo.toml index c39a783..eae5575 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "vart" publish = true -version = "0.1.1" +version = "0.1.2" edition = "2021" license = "Apache-2.0" readme = "README.md" diff --git a/src/art.rs b/src/art.rs index 7105110..4e6e424 100644 --- a/src/art.rs +++ b/src/art.rs @@ -1,11 +1,8 @@ use core::panic; use std::cmp::min; use std::ops::RangeBounds; -use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; -use hashbrown::HashSet; - use crate::iter::{Iter, Range}; use crate::node::{FlatNode, Node256, Node48, NodeTrait, TwigNode, Version}; use crate::snapshot::Snapshot; @@ -26,9 +23,6 @@ const NODE48MAX: usize = 48; // Minimum and maximum number of children for Node256 const NODE256MIN: usize = NODE48MAX + 1; -// Maximum number of active snapshots -pub const DEFAULT_MAX_ACTIVE_SNAPSHOTS: u64 = 10000; - /// A struct representing a node in an Adaptive Radix Trie. /// /// The `Node` struct encapsulates a single node within the adaptive radix trie structure. @@ -877,8 +871,7 @@ impl Node { /// A struct representing an Adaptive Radix Trie. /// /// The `Tree` struct encompasses the entire adaptive radix trie data structure. -/// It manages the root node of the tree, maintains snapshots of the tree's state, -/// and keeps track of various properties related to snapshot management. +/// It manages the root node of the tree. /// /// # Type Parameters /// @@ -888,19 +881,10 @@ impl Node { /// # Fields /// /// - `root`: An optional shared reference (using `Rc`) to the root node of the tree. -/// - `snapshots`: A `HashSet` storing snapshots of the tree's state, mapped by snapshot IDs. -/// - `max_snapshot_id`: An `AtomicU64` representing the maximum snapshot ID assigned. -/// - `max_active_snapshots`: The maximum number of active snapshots allowed. /// pub struct Tree { /// An optional shared reference to the root node of the tree. pub(crate) root: Option>>, - /// A mapping of snapshot IDs to their corresponding snapshots. - pub(crate) snapshots: HashSet, - /// An atomic value indicating the maximum snapshot ID assigned. - pub(crate) max_snapshot_id: AtomicU64, - /// The maximum number of active snapshots allowed. - pub(crate) max_active_snapshots: u64, /// A flag indicating whether the tree is closed. pub(crate) closed: bool, } @@ -951,17 +935,10 @@ impl Tree { pub fn new() -> Self { Tree { root: None, - max_snapshot_id: AtomicU64::new(0), - snapshots: HashSet::new(), - max_active_snapshots: DEFAULT_MAX_ACTIVE_SNAPSHOTS, closed: false, } } - pub fn set_max_active_snapshots(&mut self, max_active_snapshots: u64) { - self.max_active_snapshots = max_active_snapshots; - } - /// Inserts a new key-value pair with the specified version into the Trie. /// /// This function inserts a new key-value pair into the Trie. If the key already exists, @@ -991,7 +968,7 @@ impl Tree { ts: u64, ) -> Result, TrieError> { // Check if the tree is already closed - self.is_closed()?; + self.check_if_closed()?; let (new_root, old_node) = match &self.root { None => { @@ -1037,7 +1014,7 @@ impl Tree { pub fn bulk_insert(&mut self, kv_pairs: &[KV]) -> Result<(), TrieError> { // Check if the tree is already closed - self.is_closed()?; + self.check_if_closed()?; let curr_version = self.version(); let mut new_version = 0; @@ -1106,7 +1083,7 @@ impl Tree { pub fn remove(&mut self, key: &P) -> Result { // Check if the tree is already closed - self.is_closed()?; + self.check_if_closed()?; let (new_root, is_deleted) = match &self.root { None => (None, false), @@ -1130,7 +1107,7 @@ impl Tree { pub fn get(&self, key: &P, version: u64) -> Result<(P, V, u64, u64), TrieError> { // Check if the tree is already closed - self.is_closed()?; + self.check_if_closed()?; if self.root.is_none() { return Err(TrieError::Other("cannot read from empty tree".to_string())); @@ -1171,66 +1148,17 @@ impl Tree { /// Returns a `Result` containing the `Snapshot` if the snapshot is created successfully, /// or an `Err` with an appropriate error message if creation fails. /// - pub fn create_snapshot(&mut self) -> Result, TrieError> { + pub fn create_snapshot(&self) -> Result, TrieError> { // Check if the tree is already closed - self.is_closed()?; - - if self.snapshots.len() >= self.max_active_snapshots as usize { - return Err(TrieError::Other( - "max number of snapshots reached".to_string(), - )); - } - - // Increment the snapshot ID atomically - let new_snapshot_id = self.max_snapshot_id.fetch_add(1, Ordering::SeqCst); - self.snapshots.insert(new_snapshot_id); + self.check_if_closed()?; let root = self.root.as_ref().cloned(); let version = self.root.as_ref().map_or(1, |root| root.version() + 1); - let new_snapshot = Snapshot::new(new_snapshot_id, root, version); + let new_snapshot = Snapshot::new(root, version); Ok(new_snapshot) } - /// Closes a snapshot and removes it from the list of active snapshots. - /// - /// This function takes a `snapshot_id` as an argument and closes the corresponding snapshot. - /// If the snapshot exists, it is removed from the active snapshots list. If the snapshot is not - /// found, an `Err` is returned with a `TrieError::SnapshotNotFound` variant. - /// - /// # Arguments - /// - /// * `snapshot_id` - The ID of the snapshot to be closed and removed. - /// - /// # Returns - /// - /// Returns `Ok(())` if the snapshot is successfully closed and removed. Returns an `Err` - /// with `TrieError::SnapshotNotFound` if the snapshot with the given ID is not found. - /// - pub fn close_snapshot(&mut self, snapshot_id: u64) -> Result<(), TrieError> { - // Check if the tree is already closed - self.is_closed()?; - - if self.snapshots.remove(&snapshot_id) { - Ok(()) - } else { - Err(TrieError::SnapshotNotFound) - } - } - - /// Returns the count of active snapshots. - /// - /// This function returns the number of currently active snapshots in the Trie. - /// - /// # Returns - /// - /// Returns a `Result` containing the count of active snapshots if successful, or an `Err` - /// if there is an issue retrieving the snapshot count. - /// - pub fn snapshot_count(&self) -> usize { - self.snapshots.len() - } - /// Creates an iterator over the Trie's key-value pairs. /// /// This function creates and returns an iterator that can be used to traverse the key-value pairs @@ -1275,9 +1203,9 @@ impl Tree { Range::new(root, range) } - fn is_closed(&self) -> Result<(), TrieError> { + fn check_if_closed(&self) -> Result<(), TrieError> { if self.closed { - return Err(TrieError::SnapshotAlreadyClosed); + return Err(TrieError::TreeAlreadyClosed); } Ok(()) } @@ -1285,14 +1213,8 @@ impl Tree { /// Closes the tree, preventing further modifications, and releases associated resources. pub fn close(&mut self) -> Result<(), TrieError> { // Check if the tree is already closed - self.is_closed()?; - - // Check if there are any active readers for the snapshot - if self.snapshot_count() > 0 { - return Err(TrieError::SnapshotNotClosed); - } + self.check_if_closed()?; - // Mark the snapshot as closed self.closed = true; Ok(()) diff --git a/src/iter.rs b/src/iter.rs index 9e57b46..9c9c383 100644 --- a/src/iter.rs +++ b/src/iter.rs @@ -8,8 +8,6 @@ use crate::KeyTrait; // TODO: need to add more tests for snapshot readers /// A structure representing a pointer for iterating over the Trie's key-value pairs. pub struct IterationPointer { - #[allow(dead_code)] - pub(crate) id: u64, root: Arc>, } @@ -19,10 +17,9 @@ impl IterationPointer { /// # Arguments /// /// * `root` - The root node of the Trie. - /// * `id` - The ID of the snapshot. /// - pub fn new(root: Arc>, id: u64) -> IterationPointer { - IterationPointer { id, root } + pub fn new(root: Arc>) -> IterationPointer { + IterationPointer { root } } /// Returns an iterator over the key-value pairs within the Trie. diff --git a/src/lib.rs b/src/lib.rs index d5839ae..374b4c7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -387,11 +387,9 @@ pub enum TrieError { IllegalArguments, NotFound, KeyNotFound, - SnapshotNotFound, SnapshotEmpty, SnapshotNotClosed, SnapshotAlreadyClosed, - SnapshotReadersNotClosed, TreeAlreadyClosed, FixedSizeKeyLengthExceeded, Other(String), @@ -406,12 +404,8 @@ impl fmt::Display for TrieError { TrieError::IllegalArguments => write!(f, "Illegal arguments"), TrieError::NotFound => write!(f, "Not found"), TrieError::KeyNotFound => write!(f, "Key not found"), - TrieError::SnapshotNotFound => write!(f, "Snapshot not found"), TrieError::SnapshotNotClosed => write!(f, "Snapshot not closed"), TrieError::SnapshotAlreadyClosed => write!(f, "Snapshot already closed"), - TrieError::SnapshotReadersNotClosed => { - write!(f, "Readers in the snapshot are not closed") - } TrieError::TreeAlreadyClosed => write!(f, "Tree already closed"), TrieError::Other(ref message) => write!(f, "Other error: {}", message), TrieError::SnapshotEmpty => write!(f, "Snapshot is empty"), diff --git a/src/snapshot.rs b/src/snapshot.rs index f0f1efa..f0c7653 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -1,10 +1,6 @@ //! This module defines the Snapshot struct for managing snapshots within a Trie structure. -use std::sync::atomic::AtomicU64; -use std::sync::atomic::Ordering; use std::sync::Arc; -use hashbrown::HashSet; - use crate::art::Node; use crate::iter::IterationPointer; use crate::node::Version; @@ -12,23 +8,17 @@ use crate::{KeyTrait, TrieError}; /// Represents a snapshot of the data within the Trie. pub struct Snapshot { - pub(crate) id: u64, pub(crate) ts: u64, pub(crate) root: Option>>, - pub(crate) readers: HashSet, - pub(crate) max_active_readers: AtomicU64, pub(crate) closed: bool, } impl Snapshot { /// Creates a new Snapshot instance with the provided snapshot_id and root node. - pub(crate) fn new(id: u64, root: Option>>, ts: u64) -> Self { + pub(crate) fn new(root: Option>>, ts: u64) -> Self { Snapshot { - id, ts, root, - readers: HashSet::new(), - max_active_readers: AtomicU64::new(0), closed: false, } } @@ -95,10 +85,6 @@ impl Snapshot { // Check if the snapshot is already closed self.is_closed()?; - // Check if there are any active readers for the snapshot - if self.max_active_readers.load(Ordering::SeqCst) > 0 { - return Err(TrieError::SnapshotReadersNotClosed); - } // Mark the snapshot as closed self.closed = true; @@ -113,28 +99,7 @@ impl Snapshot { return Err(TrieError::SnapshotEmpty); } - let reader_id = self.max_active_readers.fetch_add(1, Ordering::SeqCst) + 1; - self.readers.insert(reader_id); - Ok(IterationPointer::new( - self.root.as_ref().unwrap().clone(), - reader_id, - )) - } - - pub fn active_readers(&self) -> Result { - // Check if the snapshot is already closed - self.is_closed()?; - - Ok(self.max_active_readers.load(Ordering::SeqCst)) - } - - pub fn close_reader(&mut self, reader_id: u64) -> Result<(), TrieError> { - // Check if the snapshot is already closed - self.is_closed()?; - - self.readers.remove(&reader_id); - let _readers = self.max_active_readers.fetch_sub(1, Ordering::SeqCst); - Ok(()) + Ok(IterationPointer::new(self.root.as_ref().unwrap().clone())) } pub fn remove(&mut self, key: &P) -> Result { @@ -164,10 +129,6 @@ impl Snapshot { pub fn ts(&self) -> u64 { self.ts } - - pub fn id(&self) -> u64 { - self.id - } } #[cfg(test)] @@ -196,7 +157,6 @@ mod tests { let expected_snap_ts = keys.len() as u64 + 1; assert_eq!(snap1.version(), expected_snap_ts); - assert_eq!(tree.snapshot_count(), 1); let expected_tree_ts = keys.len() as u64; assert_eq!(tree.version(), expected_tree_ts); @@ -214,15 +174,11 @@ mod tests { // Keys inserted before snapshot creation should be visible let mut snap1 = tree.create_snapshot().unwrap(); - assert_eq!(snap1.id, 0); assert_eq!(snap1.get(&key_1).unwrap(), (1, 1, 0)); let mut snap2 = tree.create_snapshot().unwrap(); - assert_eq!(snap2.id, 1); assert_eq!(snap2.get(&key_1).unwrap(), (1, 1, 0)); - assert_eq!(tree.snapshot_count(), 2); - // Keys inserted after snapshot creation should not be visible to other snapshots assert!(tree.insert(&key_2, 1, 0, 0).is_ok()); assert!(snap1.get(&key_2).is_err()); @@ -241,11 +197,6 @@ mod tests { assert!(snap1.close().is_ok()); assert!(snap2.close().is_ok()); - - assert!(tree.close_snapshot(snap1.id).is_ok()); - assert!(tree.close_snapshot(snap2.id).is_ok()); - - assert_eq!(tree.snapshot_count(), 0); } #[test] @@ -265,23 +216,13 @@ mod tests { // Reader 1 let reader1 = snap.new_reader().unwrap(); - let reader1_id = reader1.id; assert_eq!(count_items(&reader1), 4); - assert_eq!(reader1_id, 1); // Reader 2 let reader2 = snap.new_reader().unwrap(); - let reader2_id = reader2.id; assert_eq!(count_items(&reader2), 4); - assert_eq!(reader2_id, 2); - - // Active readers - assert_eq!(snap.active_readers().unwrap(), 2); - assert!(snap.close().is_err()); - // Close readers - assert!(snap.close_reader(reader1_id).is_ok()); - assert!(snap.close_reader(reader2_id).is_ok()); + // Close snapshot assert!(snap.close().is_ok()); }