Skip to content

feat: add multi_get() for batch point reads#274

Open
polaz wants to merge 2 commits into
fjall-rs:mainfrom
structured-world:feat/multi-get-clean
Open

feat: add multi_get() for batch point reads#274
polaz wants to merge 2 commits into
fjall-rs:mainfrom
structured-world:feat/multi-get-clean

Conversation

@polaz
Copy link
Copy Markdown

@polaz polaz commented Mar 16, 2026

Summary

  • Add multi_get() to AbstractTree trait for batch point reads
  • Default implementation calls get() in a loop; Tree and BlobTree provide optimized versions
  • BlobTree extracts a resolve_key helper for shared blob resolution logic
  • Accepts unsorted keys — callers don't need to pre-sort

Test plan

  • New integration test multi_get covering basic retrieval, snapshot isolation, missing keys, unsorted input
  • All existing tests pass
  • Full cargo test --all-features green

Closes #96

Supersedes #264 (clean rebased branch — sorry about the mess in that one).

Summary by CodeRabbit

  • New Features

    • Added a batch retrieval API to fetch multiple keys in one call, preserving input order and duplicates, and providing consistent snapshot isolation across tree structures.
  • Tests

    • Added thorough tests covering ordered/missing/duplicate keys, snapshot visibility across versions, tombstone handling, on-disk persistence, large-value (blob) indirection, and empty-input behavior.

- Add `multi_get()` to `AbstractTree` trait with default implementation
  that calls `get()` in a loop
- `Tree` and `BlobTree` provide optimized versions
- BlobTree extracts a `resolve_key` helper for shared blob resolution logic
- Accepts unsorted keys — callers don't need to pre-sort
- New integration test covering basic retrieval, snapshot isolation,
  missing keys, and unsorted input

Closes #96
Copilot AI review requested due to automatic review settings March 16, 2026 13:44
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c6bc7b24-8df4-47c8-95db-e53ba21db278

📥 Commits

Reviewing files that changed from the base of the PR and between 3acf517 and bae6679.

📒 Files selected for processing (1)
  • src/abstract_tree.rs

📝 Walkthrough

Walkthrough

Adds a batch-read API multi_get to AbstractTree and implements it for Tree and BlobTree, performing snapshot-consistent retrieval of multiple keys (preserving input order and duplicates). Includes comprehensive tests exercising memtable, disk, tombstones, snapshots, blobs, and duplicate/empty inputs.

Changes

Cohort / File(s) Summary
Trait Definition
src/abstract_tree.rs
Adds fn multi_get<K: AsRef<[u8]>>(&self, keys: impl IntoIterator<Item = K>, seqno: SeqNo) -> crate::Result<Vec<Option<UserValue>>>; to AbstractTree with a default implementation delegating to get per key.
Tree Implementation
src/tree/mod.rs
Implements multi_get for Tree: obtains a snapshot/version for seqno, iterates keys, looks up internal entries per-version, maps to UserValue, and returns results in input order.
BlobTree Implementation
src/blob_tree/mod.rs
Implements multi_get for BlobTree, refactors get to use a new resolve_key helper that resolves a key against a SuperVersion, and implements multi_get by calling resolve_key for each input key.
Tests
tests/multi_get.rs
Adds tests covering: basic batch reads, mixed present/missing keys, empty input, snapshot isolation, tombstones, reads from disk+memtable, blob KV separation, and unsorted/duplicate-key behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add multi_get() for batch point reads' clearly and concisely summarizes the primary change: adding a multi_get method for batch operations.
Linked Issues check ✅ Passed The PR successfully implements the batch point-read API requested in issue #96, with multi_get accepting keys and returning Vec<Option> with snapshot isolation support, matching the core requirements.
Out of Scope Changes check ✅ Passed All changes directly support the multi_get feature: trait definition, implementations for Tree and BlobTree, helper refactoring, and comprehensive test coverage. No unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new batch point-read API (multi_get) to the AbstractTree interface, with optimized implementations for Tree and BlobTree, plus integration tests to validate snapshot semantics and mixed-key behavior.

Changes:

  • Added AbstractTree::multi_get() with a default implementation that delegates to get() per key.
  • Implemented optimized multi_get() for Tree and BlobTree using a single snapshot (SuperVersion) per call.
  • Added an integration test suite covering existing/missing keys, tombstones, disk vs memtable, snapshot isolation, and unsorted/duplicate inputs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tests/multi_get.rs New integration tests validating multi_get correctness across key mixes, snapshots, tombstones, disk/memtable, and kv-separation.
src/tree/mod.rs Adds a Tree override of multi_get that reads from a single snapshot for all keys.
src/blob_tree/mod.rs Extracts a shared resolve_key helper and adds a BlobTree override of multi_get using a single snapshot.
src/abstract_tree.rs Introduces the multi_get API and documents snapshot/locking behavior and a usage example.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/abstract_tree.rs
Explicitly state that the returned Vec has the same length as
the input, preserves input order, returns None for missing keys,
and produces duplicate entries for duplicate input keys.
polaz added a commit to structured-world/coordinode-lsm-tree that referenced this pull request Mar 16, 2026
…s#276

Cherry-picked from upstream contribution branches:
- bae6679: document multi_get() output contract (same length,
  same order, None for missing)
- bd7cfaf: rewrite intra-L0 ordering test to exercise
  Version::with_merge directly (conflict resolved: took
  upstream's improved test that validates run position)
polaz added a commit to structured-world/coordinode-lsm-tree that referenced this pull request Mar 16, 2026
@polaz polaz requested a review from Copilot March 16, 2026 21:21
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a multi_get() batch point-read API to the AbstractTree trait, with optimized implementations for Tree and BlobTree, plus integration coverage to validate ordering/duplicates and MVCC snapshot semantics.

Changes:

  • Added AbstractTree::multi_get() with a default implementation and documented output contract (order/length/duplicates).
  • Implemented optimized multi_get() for Tree (single snapshot acquisition) and BlobTree (single snapshot + shared resolve_key helper).
  • Added tests/multi_get.rs integration tests covering existing/missing keys, empty input, snapshot isolation, tombstones, disk+memtable, blob separation, and unsorted/duplicate keys.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
tests/multi_get.rs New integration tests validating multi_get() behavior across storage layers and MVCC semantics.
src/tree/mod.rs Adds an optimized Tree::multi_get() using a single snapshot version.
src/blob_tree/mod.rs Refactors blob resolution into resolve_key() and adds BlobTree::multi_get() using a single snapshot.
src/abstract_tree.rs Adds the multi_get() API with default behavior and documented output contract + example.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/abstract_tree.rs
Comment thread src/blob_tree/mod.rs
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 22, 2026

Codecov Report

❌ Patch coverage is 86.66667% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/abstract_tree.rs 61.11% 7 Missing ⚠️
src/blob_tree/mod.rs 96.55% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multi Get

2 participants