Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
Partial batches reject displaced participants; subtree removal/replacement
refuses unsupported descendant maintenance before commit. Earlier protocol
versions retain their historical behavior.
- Under `Maintain`, a batch `DeleteTree` whose behavior declares the subtree
empty (`DontCheckWithNoCleanup`) or verifies emptiness at apply time
(`Error`, `Skip`) no longer runs a backward-reference participant scan in
full or partial batches: every element the batch removed beneath it was
already read as an old value by the batch's own deletes, so a
delete-up-tree chain costs exactly what it costs under `Skip`.
`DeleteChildren` removals and tree replacements keep their scan, and the
partial-batch observer no longer queues empty trees for scanning.
- Bumped the GroveDB workspace crates and their internal dependency requirements
to **6.0.0** for the public API changes since 5.0.1. This package version is
independent of the existing `GroveVersion` runtime compatibility versions.
Expand Down
13 changes: 10 additions & 3 deletions adr/bidirectional_references.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,14 @@ Current limitations:
specialized/indexed descendants. Remove those descendants first.
- Live participant maintenance below an indexed primary requires a full
batch; the reference cache refuses that propagation before commit.
- Recursive delete and subtree replacement inspect descendants under `Maintain`.
Those scans add reads and are charged in the V4 default cost tests.
- `DeleteChildren` removals, `Delete` of a populated tree, and subtree
replacement inspect descendants under `Maintain`; those scans add reads and
are charged in the V4 default cost tests. A `DeleteTree` whose behavior
declares the subtree empty
(`DontCheckWithNoCleanup`) or verifies emptiness at apply time (`Error`,
`Skip`) is not scanned: everything the batch removed beneath it was read as
an old value by the batch's own deletes, so a delete-up-tree chain costs
exactly what it costs under `Skip`.
- Flat drop retains its O(1) contract. Standalone `drop_flat_subtree` requires
an explicit policy argument; it and batch `DropFlat` reject `Maintain`
before scanning. Use `Skip` to acknowledge stale or dangling registrations,
Expand All @@ -116,7 +122,8 @@ transaction's original subtree contents. Cross-segment conflict checks prevent
those committed-state inspections from overlooking changes staged by the first
segment. A refusal discards the storage batch and preserves the caller's
transaction. These scans have real costs, pinned alongside the full-batch costs;
this API does not promise a no-scan recursive removal.
this API does not promise a no-scan recursive removal, only that declared-empty
and apply-time-checked `DeleteTree` removals add no reads of their own.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

## Rules

Expand Down
13 changes: 10 additions & 3 deletions docs/book/src/batch-operations.md
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,13 @@ supported by live `delete`; see the bidirectional references ADR for scope.

Under `Maintain`, ordinary batches retain their executor semantics when no old
or new value participates in backward references. Reference planner conflict
rules apply only to batches that touch participants. Recursive subtree deletion
and replacement inspect descendants and incur additional read costs.
rules apply only to batches that touch participants. `DeleteChildren`
removals and tree replacements inspect descendants and incur additional read
costs. A `DeleteTree` declared empty
(`DontCheckWithNoCleanup`) or checked empty at apply time (`Error`, `Skip`) is
not inspected: every element the batch removed beneath it already passed
through old-value observation, so a delete-up-tree chain costs the same under
`Maintain` and `Skip`.

Cost estimation follows the same split. Layers whose
`EstimatedLayerInformation` sets `may_contain_backward_references` (or use
Expand All @@ -278,4 +283,6 @@ themselves are always charged.
`BatchApplyOptions::backward_references_policy = BackwardReferencesPolicy::Skip`.
`Maintain` refuses flat drop before scanning, preserving its O(1) contract.
Partial batches refuse participant mutations in either segment; their subtree
inspection occurs before commit and can also incur recursive read costs.
inspection of `DeleteChildren` removals and tree replacements occurs before
commit and can also incur recursive read costs, with the same declared-empty
exemption.
5 changes: 4 additions & 1 deletion grovedb-version/src/version/v4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@
//! - `apply_batch.backward_references_maintenance: 1` — batches maintain
//! backward references by default: planner preparation with retained
//! old-value reads, the partial-batch participant gate, and explicit
//! `Skip` for `DropFlat`.
//! `Skip` for `DropFlat`. Removed subtrees are scanned for participants
//! only where the batch never read their contents (`DeleteChildren`,
//! tree replacement); a `DeleteTree` declared or checked empty is not
//! scanned.
//! - `apply_batch.non_merk_parent_keyed_ops_rejection: 1` — batch execution
//! refuses ordinary keyed ops at a level whose parent is a non-Merk data
//! tree (`CommitmentTree`, `MmrTree`, `BulkAppendTree`, `DenseTree`,
Expand Down
7 changes: 7 additions & 0 deletions grovedb/src/batch/backward_references.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,12 @@ pub(super) fn expand_backward_references_ops<'db>(
|| previous
.as_ref()
.is_some_and(Element::supports_backward_references);
// A removal whose contents the batch never reads needs a
// participant scan: `Delete` on a tree, `DeleteChildren`, and a tree
// replacement. A `DeleteTree` whose behavior declares the subtree
// empty (`DontCheckWithNoCleanup`) or verifies emptiness at apply
// time (`Error`, `Skip`) removes nothing that a same-batch delete
// has not already read as its old value, so it is not scanned.
let removes_subtree = !matches!(
op.op,
GroveOp::InsertIfNotExists { .. }
Expand All @@ -710,6 +716,7 @@ pub(super) fn expand_backward_references_ops<'db>(
_,
super::SubelementsDeletionBehavior::Skip
| super::SubelementsDeletionBehavior::Error
| super::SubelementsDeletionBehavior::DontCheckWithNoCleanup
)
);
if removes_subtree
Expand Down
31 changes: 30 additions & 1 deletion grovedb/src/batch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ pub enum SubelementsDeletionBehavior {
/// O(1), storage reclaimed) or [`Self::DeleteChildren`] (recursive
/// cleanup, O(contents)).
///
/// Under `BackwardReferencesPolicy::Maintain` (`GROVE_V4`+) the
/// declaration also stands in for the removed subtree's
/// backward-reference participant scan: every element the batch removed
/// beneath the tree passed through the old-value observer (or the full
/// batch's preparation), so nothing unobserved can remain and nothing is
/// read. A false declaration therefore also strands the registrations of
/// any participant it leaks, exactly like the storage.
///
/// One exception to "touches no child storage": an indexed primary
/// still gets its per-axis secondary namespaces swept, because those
/// live outside the primary's prefix and can hold stale rows even when
Expand All @@ -170,7 +178,9 @@ pub enum SubelementsDeletionBehavior {
/// still perform post-apply storage cleanup to remove the child
/// subtree's storage (and any nested subtrees), walking the structure
/// via `find_subtrees` — O(contents). Use this when the subtree may
/// contain children that should be recursively cleaned up.
/// contain children that should be recursively cleaned up. Under
/// `BackwardReferencesPolicy::Maintain` the contents are also scanned
/// for backward-reference participants before commit.
DeleteChildren,
/// Check emptiness at apply time. If the subtree is non-empty,
/// silently skip this `DeleteTree` operation (no error, no deletion).
Expand Down Expand Up @@ -4527,6 +4537,9 @@ where
}
if element.is_any_tree()
&& !element.uses_non_merk_data_storage()
&& element
.root_key_and_tree_type()
.is_some_and(|(root, _)| root.is_some())
&& (matches!(disposition, OldValueDisposition::Deleted)
|| pending_overwrite_inspections.contains_key(key))
{
Expand Down Expand Up @@ -7572,10 +7585,26 @@ impl GroveDb {
indexed_mirror_rekey_churn_bytes: continue_rekey_churn_bytes,
} = continue_captures;

// Removed subtrees whose contents the observer never saw need a
// participant scan before commit: `DeleteChildren` removals and tree
// replacements. A `DeleteTree` whose behavior declares the subtree
// empty (`DontCheckWithNoCleanup`) or verified it at apply time
// (`Error`, `Skip`) removed nothing that the batch's own deletes did
// not already pass through the observer, so it is not scanned.
for path in partial_subtree_removals
.into_iter()
.chain(continue_subtree_removals)
{
if matches!(
delete_tree_behaviors.get(&path),
Some(
SubelementsDeletionBehavior::DontCheckWithNoCleanup
| SubelementsDeletionBehavior::Error
| SubelementsDeletionBehavior::Skip
)
) {
continue;
}
if !cost_return_on_error!(
&mut cost,
self.backward_reference_participants(&path, tx.as_ref(), grove_version)
Expand Down
201 changes: 201 additions & 0 deletions grovedb/src/tests/automatic_backward_references_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -858,3 +858,204 @@ fn partial_subtree_refusal_is_atomic_in_the_continuation_and_caller_transaction(
.is_empty());
}
}

/// Delete-up-tree shape: the batch deletes a leaf, then removes the emptied
/// trees above it with `DontCheckWithNoCleanup`. Every element the batch
/// removes is read as an old value on the way, so `Maintain` has nothing
/// left to scan and adds no reads over `Skip`, in full and partial batches
/// alike. `DeleteChildren` keeps its scan: GroveDB cannot see what that
/// removal takes with it.
#[test]
fn declared_empty_subtree_removals_skip_the_participant_scan() {
use crate::batch::SubelementsDeletionBehavior;
use grovedb_merk::TreeType;
let version = GroveVersion::latest();
let seed = |db: &TempGroveDb| {
db.insert(
&[TEST_LEAF],
b"a",
Element::empty_tree(),
None,
None,
version,
)
.unwrap()
.unwrap();
db.insert(
&[TEST_LEAF, b"a"],
b"b",
Element::empty_tree(),
None,
None,
version,
)
.unwrap()
.unwrap();
db.insert(
&[TEST_LEAF, b"a", b"b"],
b"c",
Element::new_item(vec![1]),
None,
None,
version,
)
.unwrap()
.unwrap();
};
let delete_up_tree = vec![
QualifiedGroveDbOp::delete_op(
vec![TEST_LEAF.to_vec(), b"a".to_vec(), b"b".to_vec()],
b"c".to_vec(),
),
QualifiedGroveDbOp::delete_tree_op(
vec![TEST_LEAF.to_vec(), b"a".to_vec()],
b"b".to_vec(),
TreeType::NormalTree,
SubelementsDeletionBehavior::DontCheckWithNoCleanup,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
),
QualifiedGroveDbOp::delete_tree_op(
vec![TEST_LEAF.to_vec()],
b"a".to_vec(),
TreeType::NormalTree,
SubelementsDeletionBehavior::DontCheckWithNoCleanup,
),
];
let skip = Some(BatchApplyOptions {
backward_references_policy: BackwardReferencesPolicy::Skip,
..Default::default()
});
for partial in [false, true] {
let mut runs = Vec::new();
for options in [None, skip.clone()] {
let db = make_test_grovedb(version);
seed(&db);
let result = if partial {
db.apply_partial_batch(
delete_up_tree.clone(),
options,
|_, _| Ok(vec![]),
None,
version,
)
} else {
db.apply_batch(delete_up_tree.clone(), options, None, version)
};
result.value.expect("delete up the tree");
runs.push((result.cost, db.root_hash(None, version).unwrap().unwrap()));
}
let (maintain, skip_run) = (&runs[0], &runs[1]);
assert_eq!(maintain.1, skip_run.1, "partial={partial}: root hash");
assert_eq!(
maintain.0, skip_run.0,
"partial={partial}: Maintain must cost exactly what Skip costs"
);
}

let recursive = vec![QualifiedGroveDbOp::delete_tree_op(
vec![TEST_LEAF.to_vec()],
b"a".to_vec(),
TreeType::NormalTree,
SubelementsDeletionBehavior::DeleteChildren,
)];
let mut costs = Vec::new();
for options in [None, skip] {
let db = make_test_grovedb(version);
seed(&db);
let result = db.apply_batch(recursive.clone(), options, None, version);
result.value.expect("recursive delete");
costs.push(result.cost);
}
assert!(
costs[0].seek_count > costs[1].seek_count
&& costs[0].storage_loaded_bytes > costs[1].storage_loaded_bytes,
"DeleteChildren still pays for its participant scan: {:?} vs {:?}",
costs[0],
costs[1]
);
}

/// A declared-empty removal relies on the batch's own deletes for
/// maintenance: the explicitly deleted participant cascades its referrer in
/// a full batch, and a partial batch still refuses the participant mutation
/// before anything is committed.
#[test]
fn declared_empty_subtree_removal_still_maintains_the_deleted_participant() {
use crate::batch::SubelementsDeletionBehavior;
use grovedb_merk::TreeType;
let version = GroveVersion::latest();
let seed = |db: &TempGroveDb| {
db.insert(
&[TEST_LEAF],
b"tree",
Element::empty_tree(),
None,
None,
version,
)
.unwrap()
.unwrap();
db.insert(
&[TEST_LEAF, b"tree"],
b"value",
Element::new_item_allowing_bidirectional_references(vec![1]),
None,
None,
version,
)
.unwrap()
.unwrap();
let outside = Element::BidirectionalReference(
BidirectionalReference {
forward_reference_path: ReferencePathType::AbsolutePathReference(vec![
TEST_LEAF.to_vec(),
b"tree".to_vec(),
b"value".to_vec(),
]),
backward_references: Vec::new(),
cascade_on_update: true,
max_hop: None,
},
None,
);
db.insert(&[TEST_LEAF], b"outside", outside, None, None, version)
.unwrap()
.unwrap();
};
let ops = vec![
QualifiedGroveDbOp::delete_op(
vec![TEST_LEAF.to_vec(), b"tree".to_vec()],
b"value".to_vec(),
),
QualifiedGroveDbOp::delete_tree_op(
vec![TEST_LEAF.to_vec()],
b"tree".to_vec(),
TreeType::NormalTree,
SubelementsDeletionBehavior::DontCheckWithNoCleanup,
),
];

let db = make_test_grovedb(version);
seed(&db);
db.apply_batch(ops.clone(), None, None, version)
.unwrap()
.expect("the explicit participant delete cascades its referrer");
assert!(db
.get_raw(SubtreePath::from(&[TEST_LEAF]), b"outside", None, version)
.unwrap()
.is_err());
assert!(db
.get_raw(SubtreePath::from(&[TEST_LEAF]), b"tree", None, version)
.unwrap()
.is_err());
assert!(db
.verify_grovedb(None, true, true, version)
.unwrap()
.is_empty());

let db = make_test_grovedb(version);
seed(&db);
let before = db.root_hash(None, version).unwrap().unwrap();
let result = db.apply_partial_batch(ops, None, |_, _| Ok(vec![]), None, version);
assert!(matches!(result.unwrap(), Err(Error::NotSupported(_))));
assert_eq!(db.root_hash(None, version).unwrap().unwrap(), before);
}
Loading