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.

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!: just in time updates would break references in same batch #329

Merged
merged 20 commits into from
Aug 20, 2024
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
4 changes: 2 additions & 2 deletions grovedb/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ fn main() {

if !grovedbg_zip_path.exists() {
let response = reqwest::blocking::get(format!(
"https://github.com/dashpay/grovedbg/releases/download/{GROVEDBG_VERSION}/\
grovedbg-{GROVEDBG_VERSION}.zip"
"https://github.com/dashpay/grovedbg/releases/download/\
{GROVEDBG_VERSION}/grovedbg-{GROVEDBG_VERSION}.zip"
))
.expect("can't download GroveDBG artifact");

Expand Down
32 changes: 18 additions & 14 deletions grovedb/src/batch/batch_structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ use nohash_hasher::IntMap;

#[cfg(feature = "full")]
use crate::{
batch::{key_info::KeyInfo, GroveDbOp, KeyInfoPath, Op, TreeCache},
batch::{key_info::KeyInfo, GroveOp, KeyInfoPath, QualifiedGroveDbOp, TreeCache},
Element, ElementFlags, Error,
};

#[cfg(feature = "full")]
pub type OpsByPath = BTreeMap<KeyInfoPath, BTreeMap<KeyInfo, Op>>;
pub type OpsByPath = BTreeMap<KeyInfoPath, BTreeMap<KeyInfo, GroveOp>>;
/// Level, path, key, op
#[cfg(feature = "full")]
pub type OpsByLevelPath = IntMap<u32, OpsByPath>;
Expand All @@ -32,7 +32,7 @@ pub(super) struct BatchStructure<C, F, SR> {
/// Operations by level path
pub(super) ops_by_level_paths: OpsByLevelPath,
/// This is for references
pub(super) ops_by_qualified_paths: BTreeMap<Vec<Vec<u8>>, Op>,
pub(super) ops_by_qualified_paths: BTreeMap<Vec<Vec<u8>>, GroveOp>,
/// Merk trees
/// Very important: the type of run mode we are in is contained in this
/// cache
Expand Down Expand Up @@ -84,7 +84,7 @@ where
{
/// Create batch structure from a list of ops. Returns CostResult.
pub(super) fn from_ops(
ops: Vec<GroveDbOp>,
ops: Vec<QualifiedGroveDbOp>,
update_element_flags_function: F,
split_remove_bytes_function: SR,
merk_tree_cache: C,
Expand All @@ -101,7 +101,7 @@ where
/// Create batch structure from a list of ops. Returns CostResult.
pub(super) fn continue_from_ops(
previous_ops: Option<OpsByLevelPath>,
ops: Vec<GroveDbOp>,
ops: Vec<QualifiedGroveDbOp>,
update_element_flags_function: F,
split_remove_bytes_function: SR,
mut merk_tree_cache: C,
Expand All @@ -112,26 +112,30 @@ where
let mut current_last_level: u32 = 0;

// qualified paths meaning path + key
let mut ops_by_qualified_paths: BTreeMap<Vec<Vec<u8>>, Op> = BTreeMap::new();
let mut ops_by_qualified_paths: BTreeMap<Vec<Vec<u8>>, GroveOp> = BTreeMap::new();

for op in ops.into_iter() {
let mut path = op.path.clone();
path.push(op.key.clone());
ops_by_qualified_paths.insert(path.to_path_consume(), op.op.clone());
let op_cost = OperationCost::default();
let op_result = match &op.op {
Op::Insert { element } | Op::Replace { element } | Op::Patch { element, .. } => {
GroveOp::InsertOnly { element }
| GroveOp::InsertOrReplace { element }
| GroveOp::Replace { element }
| GroveOp::Patch { element, .. } => {
if let Element::Tree(..) = element {
cost_return_on_error!(&mut cost, merk_tree_cache.insert(&op, false));
} else if let Element::SumTree(..) = element {
cost_return_on_error!(&mut cost, merk_tree_cache.insert(&op, true));
}
Ok(())
}
Op::RefreshReference { .. } | Op::Delete | Op::DeleteTree | Op::DeleteSumTree => {
Ok(())
}
Op::ReplaceTreeRootKey { .. } | Op::InsertTreeWithRootHash { .. } => {
GroveOp::RefreshReference { .. }
| GroveOp::Delete
| GroveOp::DeleteTree
| GroveOp::DeleteSumTree => Ok(()),
GroveOp::ReplaceTreeRootKey { .. } | GroveOp::InsertTreeWithRootHash { .. } => {
Err(Error::InvalidBatchOperation(
"replace and insert tree hash are internal operations only",
))
Expand All @@ -146,14 +150,14 @@ where
if let Some(ops_on_path) = ops_on_level.get_mut(&op.path) {
ops_on_path.insert(op.key, op.op);
} else {
let mut ops_on_path: BTreeMap<KeyInfo, Op> = BTreeMap::new();
let mut ops_on_path: BTreeMap<KeyInfo, GroveOp> = BTreeMap::new();
ops_on_path.insert(op.key, op.op);
ops_on_level.insert(op.path.clone(), ops_on_path);
}
} else {
let mut ops_on_path: BTreeMap<KeyInfo, Op> = BTreeMap::new();
let mut ops_on_path: BTreeMap<KeyInfo, GroveOp> = BTreeMap::new();
ops_on_path.insert(op.key, op.op);
let mut ops_on_level: BTreeMap<KeyInfoPath, BTreeMap<KeyInfo, Op>> =
let mut ops_on_level: BTreeMap<KeyInfoPath, BTreeMap<KeyInfo, GroveOp>> =
BTreeMap::new();
ops_on_level.insert(op.path, ops_on_path);
ops_by_level_paths.insert(level, ops_on_level);
Expand Down
60 changes: 31 additions & 29 deletions grovedb/src/batch/estimated_costs/average_case_costs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@
#[cfg(feature = "full")]
use crate::{
batch::{
key_info::KeyInfo, mode::BatchRunMode, BatchApplyOptions, GroveDbOp, KeyInfoPath, Op,
TreeCache,
key_info::KeyInfo, mode::BatchRunMode, BatchApplyOptions, GroveOp, KeyInfoPath,
QualifiedGroveDbOp, TreeCache,
},
Error, GroveDb,
};

#[cfg(feature = "full")]
impl Op {
impl GroveOp {
/// Get the estimated average case cost of the op. Calls a lower level
/// function to calculate the estimate based on the type of op. Returns
/// CostResult.
Expand All @@ -53,14 +53,14 @@
}
};
match self {
Op::ReplaceTreeRootKey { sum, .. } => GroveDb::average_case_merk_replace_tree(
GroveOp::ReplaceTreeRootKey { sum, .. } => GroveDb::average_case_merk_replace_tree(
key,
layer_element_estimates,
sum.is_some(),
propagate,
grove_version,
),
Op::InsertTreeWithRootHash { flags, sum, .. } => {
GroveOp::InsertTreeWithRootHash { flags, sum, .. } => {
GroveDb::average_case_merk_insert_tree(
key,
flags,
Expand All @@ -70,14 +70,16 @@
grove_version,
)
}
Op::Insert { element } => GroveDb::average_case_merk_insert_element(
key,
element,
in_tree_using_sums,
propagate_if_input(),
grove_version,
),
Op::RefreshReference {
GroveOp::InsertOrReplace { element } | GroveOp::InsertOnly { element } => {
GroveDb::average_case_merk_insert_element(
key,
element,
in_tree_using_sums,
propagate_if_input(),
grove_version,
)
}
GroveOp::RefreshReference {
reference_path_type,
max_reference_hop,
flags,
Expand All @@ -93,14 +95,14 @@
propagate_if_input(),
grove_version,
),
Op::Replace { element } => GroveDb::average_case_merk_replace_element(
GroveOp::Replace { element } => GroveDb::average_case_merk_replace_element(
key,
element,
in_tree_using_sums,
propagate_if_input(),
grove_version,
),
Op::Patch {
GroveOp::Patch {
element,
change_in_bytes,
} => GroveDb::average_case_merk_patch_element(
Expand All @@ -111,20 +113,20 @@
propagate_if_input(),
grove_version,
),
Op::Delete => GroveDb::average_case_merk_delete_element(
GroveOp::Delete => GroveDb::average_case_merk_delete_element(
key,
layer_element_estimates,
propagate,
grove_version,
),
Op::DeleteTree => GroveDb::average_case_merk_delete_tree(
GroveOp::DeleteTree => GroveDb::average_case_merk_delete_tree(
key,
false,
layer_element_estimates,
propagate,
grove_version,
),
Op::DeleteSumTree => GroveDb::average_case_merk_delete_tree(
GroveOp::DeleteSumTree => GroveDb::average_case_merk_delete_tree(
key,
true,
layer_element_estimates,
Expand Down Expand Up @@ -165,7 +167,7 @@

#[cfg(feature = "full")]
impl<G, SR> TreeCache<G, SR> for AverageCaseTreeCacheKnownPaths {
fn insert(&mut self, op: &GroveDbOp, is_sum_tree: bool) -> CostResult<(), Error> {
fn insert(&mut self, op: &QualifiedGroveDbOp, is_sum_tree: bool) -> CostResult<(), Error> {
let mut average_case_cost = OperationCost::default();
let mut inserted_path = op.path.clone();
inserted_path.push(op.key.clone());
Expand All @@ -184,8 +186,8 @@
fn execute_ops_on_path(
&mut self,
path: &KeyInfoPath,
ops_at_path_by_key: BTreeMap<KeyInfo, Op>,
_ops_by_qualified_paths: &BTreeMap<Vec<Vec<u8>>, Op>,
ops_at_path_by_key: BTreeMap<KeyInfo, GroveOp>,
_ops_by_qualified_paths: &BTreeMap<Vec<Vec<u8>>, GroveOp>,
_batch_apply_options: &BatchApplyOptions,
_flags_update: &mut G,
_split_removal_bytes: &mut SR,
Expand Down Expand Up @@ -214,7 +216,7 @@
.estimated_to_be_empty();

// Then we have to get the tree
if self.cached_merks.get(path).is_none() {

Check warning on line 219 in grovedb/src/batch/estimated_costs/average_case_costs.rs

View workflow job for this annotation

GitHub Actions / clippy

unnecessary use of `get(path).is_none()`

warning: unnecessary use of `get(path).is_none()` --> grovedb/src/batch/estimated_costs/average_case_costs.rs:219:30 | 219 | if self.cached_merks.get(path).is_none() { | ------------------^^^^^^^^^^^^^^^^^^^ | | | help: replace it with: `!self.cached_merks.contains_key(path)` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_get_then_check = note: `#[warn(clippy::unnecessary_get_then_check)]` on by default
let layer_info = cost_return_on_error_no_add!(
&cost,
self.paths.get(path).ok_or_else(|| {
Expand Down Expand Up @@ -268,22 +270,22 @@
let base_path = KeyInfoPath(vec![]);
if let Some(estimated_layer_info) = self.paths.get(&base_path) {
// Then we have to get the tree
if !self.cached_merks.contains_key(&base_path) {
cost_return_on_error_no_add!(
&cost,
GroveDb::add_average_case_get_merk_at_path::<RocksDbStorage>(
&mut cost,
&base_path,
estimated_layer_info
.estimated_layer_count
.estimated_to_be_empty(),
estimated_layer_info.is_sum_tree,
grove_version
)
);
self.cached_merks
.insert(base_path, estimated_layer_info.is_sum_tree);
}

Check warning on line 288 in grovedb/src/batch/estimated_costs/average_case_costs.rs

View workflow job for this annotation

GitHub Actions / clippy

usage of `contains_key` followed by `insert` on a `HashMap`

warning: usage of `contains_key` followed by `insert` on a `HashMap` --> grovedb/src/batch/estimated_costs/average_case_costs.rs:273:13 | 273 | / if !self.cached_merks.contains_key(&base_path) { 274 | | cost_return_on_error_no_add!( 275 | | &cost, 276 | | GroveDb::add_average_case_get_merk_at_path::<RocksDbStorage>( ... | 287 | | .insert(base_path, estimated_layer_info.is_sum_tree); 288 | | } | |_____________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_entry = note: `#[warn(clippy::map_entry)]` on by default help: try | 273 ~ if let std::collections::hash_map::Entry::Vacant(e) = self.cached_merks.entry(base_path) { 274 + cost_return_on_error_no_add!( 275 + &cost, 276 + GroveDb::add_average_case_get_merk_at_path::<RocksDbStorage>( 277 + &mut cost, 278 + &base_path, 279 + estimated_layer_info 280 + .estimated_layer_count 281 + .estimated_to_be_empty(), 282 + estimated_layer_info.is_sum_tree, 283 + grove_version 284 + ) 285 + ); 286 + e.insert(estimated_layer_info.is_sum_tree); 287 + } |
}
Ok(()).wrap_with_cost(cost)
}
Expand All @@ -309,7 +311,7 @@
use crate::{
batch::{
estimated_costs::EstimatedCostsType::AverageCaseCostsType, key_info::KeyInfo,
GroveDbOp, KeyInfoPath,
KeyInfoPath, QualifiedGroveDbOp,
},
tests::{common::EMPTY_PATH, make_empty_grovedb},
Element, GroveDb,
Expand All @@ -321,7 +323,7 @@
let db = make_empty_grovedb();
let tx = db.start_transaction();

let ops = vec![GroveDbOp::insert_op(
let ops = vec![QualifiedGroveDbOp::insert_or_replace_op(
vec![],
b"key1".to_vec(),
Element::empty_tree(),
Expand Down Expand Up @@ -390,7 +392,7 @@
let db = make_empty_grovedb();
let tx = db.start_transaction();

let ops = vec![GroveDbOp::insert_op(
let ops = vec![QualifiedGroveDbOp::insert_or_replace_op(
vec![],
b"key1".to_vec(),
Element::empty_tree_with_flags(Some(b"cat".to_vec())),
Expand Down Expand Up @@ -457,7 +459,7 @@
let db = make_empty_grovedb();
let tx = db.start_transaction();

let ops = vec![GroveDbOp::insert_op(
let ops = vec![QualifiedGroveDbOp::insert_or_replace_op(
vec![],
b"key1".to_vec(),
Element::new_item(b"cat".to_vec()),
Expand Down Expand Up @@ -530,7 +532,7 @@
.unwrap()
.expect("successful root tree leaf insert");

let ops = vec![GroveDbOp::insert_op(
let ops = vec![QualifiedGroveDbOp::insert_or_replace_op(
vec![],
b"key1".to_vec(),
Element::empty_tree(),
Expand Down Expand Up @@ -616,7 +618,7 @@
.unwrap()
.expect("successful root tree leaf insert");

let ops = vec![GroveDbOp::insert_op(
let ops = vec![QualifiedGroveDbOp::insert_or_replace_op(
vec![b"0".to_vec()],
b"key1".to_vec(),
Element::empty_tree(),
Expand Down Expand Up @@ -695,7 +697,7 @@
#[test]
fn test_batch_root_one_sum_item_replace_op_average_case_costs() {
let grove_version = GroveVersion::latest();
let ops = vec![GroveDbOp::replace_op(
let ops = vec![QualifiedGroveDbOp::replace_op(
vec![vec![7]],
hex::decode("46447a3b4c8939fd4cf8b610ba7da3d3f6b52b39ab2549bf91503b9b07814055")
.unwrap(),
Expand Down Expand Up @@ -774,7 +776,7 @@
.unwrap()
.expect("successful root tree leaf insert");

let ops = vec![GroveDbOp::insert_op(
let ops = vec![QualifiedGroveDbOp::insert_or_replace_op(
vec![],
b"key1".to_vec(),
Element::empty_tree(),
Expand Down
Loading
Loading