-
Notifications
You must be signed in to change notification settings - Fork 27
feat: paginated position-range reads with proofs at the BulkAppendTree layer #786
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
Changes from 7 commits
5692653
b473431
9ae0377
3f3efe2
ad8c8a7
9326581
eac3480
acb99bd
9584bdb
7c038fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ use grovedb_merkle_mountain_range::{leaf_to_pos, MmrKeySize, MmrStore, MMR}; | |
| use grovedb_query::Query; | ||
| use grovedb_storage::StorageContext; | ||
|
|
||
| use super::BulkAppendTree; | ||
| use super::{BulkAppendTree, RangePage}; | ||
| use crate::{chunk::deserialize_chunk_blob, BulkAppendError}; | ||
|
|
||
| /// Result of querying the dense tree buffer. | ||
|
|
@@ -63,6 +63,90 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree<S> { | |
| Ok(BufferQueryResult { entries, proof }) | ||
| } | ||
|
|
||
| // ── Range operations (chunks + buffer) ─────────────────────────── | ||
|
|
||
| /// Fetch entries for the position range `[start, start + limit)`, | ||
| /// clamped to the tree's total count. | ||
| /// | ||
| /// This is the paginated-scan read path: clients walking "all entries | ||
| /// since my cursor" call it with their cursor as `start` and advance by | ||
| /// `entries.len()`. The read is chunk-aligned — each completed chunk | ||
| /// overlapping the range is read and deserialized exactly once, so a | ||
| /// page costs O(chunks touched) blob reads plus one read per buffer | ||
| /// entry, not O(entries) random reads. | ||
| /// | ||
| /// Absence needs no lookup: positions `>= total_count` do not exist, so | ||
| /// a page shorter than `limit` means the end of the tree was reached. | ||
| pub fn get_range(&self, start: u64, limit: u16) -> Result<RangePage, BulkAppendError> { | ||
| let total_count = self.total_count; | ||
| let end = start.saturating_add(limit as u64).min(total_count); | ||
| if start >= end { | ||
| return Ok(RangePage { | ||
| entries: Vec::new(), | ||
| total_count, | ||
| }); | ||
| } | ||
| let mut entries = Vec::with_capacity((end - start) as usize); | ||
|
|
||
| let epoch_size = self.epoch_size(); | ||
| let buffer_start = self.chunk_count() * epoch_size; | ||
|
|
||
| // Completed chunks overlapping [start, min(end, buffer_start)). | ||
| // The MMR (with its overlay clone) is built once and reused for every | ||
| // chunk in the page — going through `get_chunk_value` would rebuild | ||
| // it, and re-clone the overlay, per chunk. | ||
| let chunk_end = end.min(buffer_start); | ||
| if start < chunk_end { | ||
| let first_chunk = start / epoch_size; | ||
| let last_chunk = (chunk_end - 1) / epoch_size; | ||
| let mmr_store = MmrStore::with_key_size(&self.dense_tree.storage, MmrKeySize::U32); | ||
| let mmr = MMR::new_with_overlay(self.mmr_size(), &mmr_store, self.mmr_overlay.clone()); | ||
| for chunk_idx in first_chunk..=last_chunk { | ||
| let node = mmr | ||
| .batch | ||
| .element_at_position(leaf_to_pos(chunk_idx)) | ||
| .unwrap() | ||
| .map_err(|e| { | ||
| BulkAppendError::MmrError(format!( | ||
| "failed to read MMR node for chunk {}: {}", | ||
| chunk_idx, e | ||
| )) | ||
| })?; | ||
| let blob = node.and_then(|n| n.into_value()).ok_or_else(|| { | ||
| BulkAppendError::CorruptedData(format!( | ||
| "missing chunk blob for index {}", | ||
| chunk_idx | ||
| )) | ||
| })?; | ||
| let chunk_entries = deserialize_chunk_blob(&blob)?; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Validate completed chunk length
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is Claude. Fixed in acb99bd: |
||
| let chunk_start = chunk_idx * epoch_size; | ||
| for (i, value) in chunk_entries.into_iter().enumerate() { | ||
| let pos = chunk_start + i as u64; | ||
| if pos >= start && pos < chunk_end { | ||
| entries.push((pos, value)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Buffer tail: positions in [max(start, buffer_start), end) | ||
| for pos in start.max(buffer_start)..end { | ||
| let buffer_pos = (pos - buffer_start) as u16; | ||
| let value = self.get_buffer_value(buffer_pos)?.ok_or_else(|| { | ||
| BulkAppendError::CorruptedData(format!( | ||
| "missing buffer value at position {}", | ||
| buffer_pos | ||
| )) | ||
| })?; | ||
| entries.push((pos, value)); | ||
| } | ||
|
|
||
| Ok(RangePage { | ||
| entries, | ||
| total_count, | ||
| }) | ||
| } | ||
|
|
||
| // ── Chunk operations (MMR) ─────────────────────────────────────── | ||
|
|
||
| /// Get a single completed chunk's raw blob by chunk index. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Preserve range-read storage costs
get_rangereturns a plainResult, so the MMRelement_at_position(...).unwrap()and every buffergetdiscard theirCostResult.cost. Consequently,bulk_get_rangeandcommitment_tree_get_rangereport none of the page's seeks or loaded bytes—up to 65,535 uncharged buffer reads. Please return and aggregate aCostResultthrough both GroveDB wrappers so cost limits reflect the actual work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is Claude. Fixed in acb99bd:
get_rangenow returns aCostResult— the chunk-MMR node reads and every dense-buffer read charge their seeks and loaded bytes (buffer reads go throughdense_tree.getdirectly so nothing is discarded), and the costs aggregate throughCommitmentTree::get_range,bulk_get_range, andcommitment_tree_get_range. Addedtest_bulk_get_range_reports_storage_costs, which asserts nonzero seek/loaded-byte costs on a chunk+buffer-spanning page and that an 11-entry page loads more bytes than a 1-entry page.