Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix/refactor: atomicity and caching #347

Merged
merged 6 commits into from
Jan 21, 2025
Merged

fix/refactor: atomicity and caching #347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4b58b52
wip
fominok Jan 17, 2025
57e3560
fix
fominok Jan 17, 2025
5165950
Merge remote-tracking branch 'origin/develop' into cleanup-non-transa…
fominok Jan 21, 2025
338d7e7
revert to pub
fominok Jan 21, 2025
c7e2481
fix
fominok Jan 21, 2025
5d4d504
address clippy warnings
fominok Jan 21, 2025
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
16 changes: 13 additions & 3 deletions costs/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,15 @@ impl<T, E> CostResult<T, E> {
pub fn cost_as_result(self) -> Result<OperationCost, E> {
self.value.map(|_| self.cost)
}

/// Call the provided function on success without altering result or cost.
pub fn for_ok(self, f: impl FnOnce(&T)) -> CostResult<T, E> {
if let Ok(x) = &self.value {
f(x)
}

self
}
}

impl<T, E> CostResult<Result<T, E>, E> {
Expand Down Expand Up @@ -170,8 +179,9 @@ impl<T> CostsExt for T {}
/// 1. Early termination on error;
/// 2. Because of 1, `Result` is removed from the equation;
/// 3. `CostContext` is removed too because it is added to external cost
/// accumulator; 4. Early termination uses external cost accumulator so previous
/// costs won't be lost.
/// accumulator;
/// 4. Early termination uses external cost accumulator so previous costs won't
/// be lost.
#[macro_export]
macro_rules! cost_return_on_error {
( &mut $cost:ident, $($body:tt)+ ) => {
Expand All @@ -193,7 +203,7 @@ macro_rules! cost_return_on_error {
/// so no costs will be added except previously accumulated.
#[macro_export]
macro_rules! cost_return_on_error_no_add {
( &$cost:ident, $($body:tt)+ ) => {
( $cost:ident, $($body:tt)+ ) => {
{
use $crate::CostsExt;
let result = { $($body)+ };
Expand Down
38 changes: 21 additions & 17 deletions grovedb-version/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::version::GroveVersion;
use version::GroveVersion;

pub mod error;
pub mod version;
Expand All @@ -8,13 +8,15 @@ macro_rules! check_grovedb_v0_with_cost {
($method:expr, $version:expr) => {{
const EXPECTED_VERSION: u16 = 0;
if $version != EXPECTED_VERSION {
return Err(GroveVersionError::UnknownVersionMismatch {
method: $method.to_string(),
known_versions: vec![EXPECTED_VERSION],
received: $version,
}
.into())
.wrap_with_cost(OperationCost::default());
return grovedb_costs::CostsExt::wrap_with_cost(
Err($crate::error::GroveVersionError::UnknownVersionMismatch {
method: $method.to_string(),
known_versions: vec![EXPECTED_VERSION],
received: $version,
}
.into()),
Default::default(),
);
}
}};
}
Expand All @@ -24,7 +26,7 @@ macro_rules! check_grovedb_v0 {
($method:expr, $version:expr) => {{
const EXPECTED_VERSION: u16 = 0;
if $version != EXPECTED_VERSION {
return Err(GroveVersionError::UnknownVersionMismatch {
return Err($crate::error::GroveVersionError::UnknownVersionMismatch {
method: $method.to_string(),
known_versions: vec![EXPECTED_VERSION],
received: $version,
Expand Down Expand Up @@ -56,13 +58,15 @@ macro_rules! check_merk_v0_with_cost {
($method:expr, $version:expr) => {{
const EXPECTED_VERSION: u16 = 0;
if $version != EXPECTED_VERSION {
return Err(GroveVersionError::UnknownVersionMismatch {
method: $method.to_string(),
known_versions: vec![EXPECTED_VERSION],
received: $version,
}
.into())
.wrap_with_cost(OperationCost::default());
return grovedb_costs::CostsExt::wrap_with_cost(
Err($crate::error::GroveVersionError::UnknownVersionMismatch {
method: $method.to_string(),
known_versions: vec![EXPECTED_VERSION],
received: $version,
}
.into()),
Default::default(),
);
}
}};
}
Expand All @@ -72,7 +76,7 @@ macro_rules! check_merk_v0 {
($method:expr, $version:expr) => {{
const EXPECTED_VERSION: u16 = 0;
if $version != EXPECTED_VERSION {
return Err(GroveVersionError::UnknownVersionMismatch {
return Err($crate::error::GroveVersionError::UnknownVersionMismatch {
method: $method.to_string(),
known_versions: vec![EXPECTED_VERSION],
received: $version,
Expand Down
3 changes: 3 additions & 0 deletions grovedb-version/src/version/grovedb_versions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub struct GroveDBOperationsGetVersions {
pub get: FeatureVersion,
pub get_caching_optional: FeatureVersion,
pub follow_reference: FeatureVersion,
pub follow_reference_once: FeatureVersion,
pub get_raw: FeatureVersion,
pub get_raw_caching_optional: FeatureVersion,
pub get_raw_optional: FeatureVersion,
Expand Down Expand Up @@ -190,6 +191,7 @@ pub struct GroveDBElementMethodVersions {
pub get_optional_from_storage: FeatureVersion,
pub get_with_absolute_refs: FeatureVersion,
pub get_value_hash: FeatureVersion,
pub get_with_value_hash: FeatureVersion,
pub get_specialized_cost: FeatureVersion,
pub value_defined_cost: FeatureVersion,
pub value_defined_cost_for_serialized_value: FeatureVersion,
Expand All @@ -203,6 +205,7 @@ pub struct GroveDBElementMethodVersions {
pub insert_if_changed_value_into_batch_operations: FeatureVersion,
pub insert_reference: FeatureVersion,
pub insert_reference_into_batch_operations: FeatureVersion,
pub insert_reference_if_changed_value: FeatureVersion,
pub insert_subtree: FeatureVersion,
pub insert_subtree_into_batch_operations: FeatureVersion,
pub get_query: FeatureVersion,
Expand Down
3 changes: 3 additions & 0 deletions grovedb-version/src/version/v1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ pub const GROVE_V1: GroveVersion = GroveVersion {
serialize: 0,
serialized_size: 0,
deserialize: 0,
get_with_value_hash: 0,
insert_reference_if_changed_value: 0,
},
operations: GroveDBOperationsVersions {
get: GroveDBOperationsGetVersions {
Expand All @@ -86,6 +88,7 @@ pub const GROVE_V1: GroveVersion = GroveVersion {
worst_case_for_get_raw: 0,
worst_case_for_get: 0,
is_empty_tree: 0,
follow_reference_once: 0,
},
insert: GroveDBOperationsInsertVersions {
insert: 0,
Expand Down
3 changes: 3 additions & 0 deletions grovedb-version/src/version/v2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ pub const GROVE_V2: GroveVersion = GroveVersion {
serialize: 0,
serialized_size: 0,
deserialize: 0,
get_with_value_hash: 0,
insert_reference_if_changed_value: 0,
},
operations: GroveDBOperationsVersions {
get: GroveDBOperationsGetVersions {
Expand All @@ -86,6 +88,7 @@ pub const GROVE_V2: GroveVersion = GroveVersion {
worst_case_for_get_raw: 0,
worst_case_for_get: 0,
is_empty_tree: 0,
follow_reference_once: 0,
},
insert: GroveDBOperationsInsertVersions {
insert: 0,
Expand Down
12 changes: 7 additions & 5 deletions grovedb/src/batch/estimated_costs/average_case_costs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ impl<G, SR> TreeCache<G, SR> for AverageCaseTreeCacheKnownPaths {
let mut cost = OperationCost::default();

let layer_element_estimates = cost_return_on_error_no_add!(
&cost,
cost,
self.paths.get(path).ok_or_else(|| {
let paths = self
.paths
Expand All @@ -212,9 +212,9 @@ impl<G, SR> TreeCache<G, SR> for AverageCaseTreeCacheKnownPaths {
.estimated_to_be_empty();

// Then we have to get the tree
if self.cached_merks.get(path).is_none() {
if !self.cached_merks.contains_key(path) {
let layer_info = cost_return_on_error_no_add!(
&cost,
cost,
self.paths.get(path).ok_or_else(|| {
let paths = self
.paths
Expand All @@ -229,7 +229,7 @@ impl<G, SR> TreeCache<G, SR> for AverageCaseTreeCacheKnownPaths {
})
);
cost_return_on_error_no_add!(
&cost,
cost,
GroveDb::add_average_case_get_merk_at_path::<RocksDbStorage>(
&mut cost,
path,
Expand All @@ -256,6 +256,8 @@ impl<G, SR> TreeCache<G, SR> for AverageCaseTreeCacheKnownPaths {
Ok(([0u8; 32], None, AggregateData::NoAggregateData)).wrap_with_cost(cost)
}

// Clippy's suggestion doesn't respect ownership in this case
#[allow(clippy::map_entry)]
fn update_base_merk_root_key(
&mut self,
_root_key: Option<Vec<u8>>,
Expand All @@ -268,7 +270,7 @@ impl<G, SR> TreeCache<G, SR> for AverageCaseTreeCacheKnownPaths {
// Then we have to get the tree
if !self.cached_merks.contains_key(&base_path) {
cost_return_on_error_no_add!(
&cost,
cost,
GroveDb::add_average_case_get_merk_at_path::<RocksDbStorage>(
&mut cost,
&base_path,
Expand Down
6 changes: 3 additions & 3 deletions grovedb/src/batch/estimated_costs/worst_case_costs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ impl<G, SR> TreeCache<G, SR> for WorstCaseTreeCacheKnownPaths {
let mut cost = OperationCost::default();

let worst_case_layer_element_estimates = cost_return_on_error_no_add!(
&cost,
cost,
self.paths
.get(path)
.ok_or_else(|| Error::PathNotFoundInCacheForEstimatedCosts(format!(
Expand All @@ -201,7 +201,7 @@ impl<G, SR> TreeCache<G, SR> for WorstCaseTreeCacheKnownPaths {
// Then we have to get the tree
if !self.cached_merks.contains(path) {
cost_return_on_error_no_add!(
&cost,
cost,
GroveDb::add_worst_case_get_merk_at_path::<RocksDbStorage>(
&mut cost,
path,
Expand Down Expand Up @@ -244,7 +244,7 @@ impl<G, SR> TreeCache<G, SR> for WorstCaseTreeCacheKnownPaths {
// Then we have to get the tree
if !self.cached_merks.contains(&base_path) {
cost_return_on_error_no_add!(
&cost,
cost,
GroveDb::add_worst_case_get_merk_at_path::<RocksDbStorage>(
&mut cost,
&base_path,
Expand Down
16 changes: 7 additions & 9 deletions grovedb/src/batch/just_in_time_reference_update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@
F: FnMut(&[Vec<u8>], bool) -> CostResult<Merk<S>, Error>,
S: StorageContext<'db>,
{
pub(crate) fn process_old_element_flags<G, SR>(
key: &[u8],
serialized: &[u8],
new_element: &mut Element,
old_element: Element,
old_serialized_element: &[u8],
in_tree_type: TreeType,
flags_update: &mut G,
split_removal_bytes: &mut SR,
grove_version: &GroveVersion,
) -> CostResult<CryptoHash, Error>

Check warning on line 39 in grovedb/src/batch/just_in_time_reference_update.rs

View workflow job for this annotation

GitHub Actions / clippy

this function has too many arguments (9/7) 

warning: this function has too many arguments (9/7)
  --> grovedb/src/batch/just_in_time_reference_update.rs:29:5
   |
29 | /     pub(crate) fn process_old_element_flags<G, SR>(
30 | |         key: &[u8],
31 | |         serialized: &[u8],
32 | |         new_element: &mut Element,
...  |
38 | |         grove_version: &GroveVersion,
39 | |     ) -> CostResult<CryptoHash, Error>
   | |______________________________________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
   = note: `#[warn(clippy::too_many_arguments)]` on by default
where
G: FnMut(&StorageCost, Option<ElementFlags>, &mut ElementFlags) -> Result<bool, Error>,
SR: FnMut(
Expand All @@ -54,13 +54,13 @@
updated_new_element_with_old_flags.set_flags(maybe_old_flags.clone());
// There are no storage flags, we can just hash new element
let new_serialized_bytes = cost_return_on_error_no_add!(
&cost,
cost,
updated_new_element_with_old_flags.serialize(grove_version)
);
let val_hash = value_hash(&new_serialized_bytes).unwrap_add_cost(&mut cost);
Ok(val_hash).wrap_with_cost(cost)
} else {
let val_hash = value_hash(&serialized).unwrap_add_cost(&mut cost);
let val_hash = value_hash(serialized).unwrap_add_cost(&mut cost);
Ok(val_hash).wrap_with_cost(cost)
}
} else {
Expand Down Expand Up @@ -94,7 +94,7 @@
updated_new_element_with_old_flags.set_flags(maybe_old_flags.clone());

let serialized_with_old_flags = cost_return_on_error_no_add!(
&cost,
cost,
updated_new_element_with_old_flags.serialize(grove_version)
);
KV::node_value_byte_cost_size(
Expand All @@ -120,7 +120,7 @@
if let Some(old_element_flags) = maybe_old_flags.as_mut() {
if let BasicStorageRemoval(removed_bytes) = storage_costs.removed_bytes {
let (_, value_removed_bytes) = cost_return_on_error_no_add!(
&cost,
cost,
split_removal_bytes(old_element_flags, 0, removed_bytes)
);
storage_costs.removed_bytes = value_removed_bytes;
Expand All @@ -130,7 +130,7 @@
let mut new_element_cloned = original_new_element.clone();

let changed = cost_return_on_error_no_add!(
&cost,
cost,
(flags_update)(
&storage_costs,
maybe_old_flags.clone(),
Expand All @@ -150,10 +150,8 @@
return Ok(val_hash).wrap_with_cost(cost);
} else {
// There are no storage flags, we can just hash new element
let new_serialized_bytes = cost_return_on_error_no_add!(
&cost,
new_element_cloned.serialize(grove_version)
);
let new_serialized_bytes =
cost_return_on_error_no_add!(cost, new_element_cloned.serialize(grove_version));

new_storage_cost = KV::node_value_byte_cost_size(
key.len() as u32,
Expand Down
Loading
Loading