Skip to content

collections: reuse captured physical leaf workspace on native reads - #5188

Merged
snissn merged 3 commits into
mainfrom
codex/native-leaf-reuse-5181-20261020
Oct 20, 2026
Merged

snissn merged 3 commits into
mainfrom
codex/native-leaf-reuse-5181-20261020

Conversation

@snissn

@snissn snissn commented Oct 20, 2026 •

Copy link
Copy Markdown
Owner

Ordinary native collection updates and index traversal currently rebuild physical leaf decoding state for reads already owned by a retained COW capture. Reuse the existing admitted physical-reader workspace through an optional leaf-reader capability, copying decoded bytes into the caller’s admitted destination while retaining snapshot identity, checksum/codec/inspection, capacity, fallback and Close rules.

The reader change is four paths on main and adds no owner, pool, cache, format or protocol. It includes no document-vector feature, kind105 or DATA retirement changes. Final 67aee9e includes merged CI repair #5187 and one discovery-inventory refresh. Its four reader postimages remain identical to 48bec; its complete tree is identical to locally tested 695f6d.

Fresh main03 and reader48bec binaries ran all five existing public32-document UpdateBatch+Checkpoint cells once at8x/count1:

Cell B/op main→reader allocs/op main→reader time change
JSON/scalar 757409→649783 2467→2149 -3.74%
JSON/multikey 1246403→993149 3892→3415 -3.35%
BSON/scalar 683245→574446 2291→1976 -2.42%
BSON/multikey 1531360→1241296 4805→4327 +1.29%
template-v1/scalar 676091→589442 2545→2219 -1.24%

These single pairs are directional, with the adverse BSON/multikey time retained. Each operation includes the original checkpoint; original activation, logical sequence, exact documents/index owners and final-zero COW resources passed. Public UpdateBatch reaches the changed physical readLeaf path through touchNativeRow after checkpoint; public Get alone uses an existing shortcut. Earlier donor document-vector measurements are not evidence for this delivery.

On695f6d, seven existing public JSON/BSON/multikey/template/retained-update/compound/owned-fetch controls passed (20 subtests, no skips). The original five reader controls and public scalar range lifecycle passed previously at production-equivalent48bec (six tops/seven subtests). Ten changed-risk race controls passed on695f6d (21 subtests, no skips). Full owning collections passed on 695f6d: 3,633 tests plus two fuzz seed groups, 2,746 subtests, ten separate skips and zero failures (529 seconds). Final67aee9e has the identical complete tree; executed-head identity is retained, with no local rerun. The remaining latest-head hosted checks are running; automatic Codex review of final 67aee9e is clean, with zero review threads. The CI predecessor has landed and exact final-base ancestry is recorded by root. The future14 document-vector controls, broader Q and final-engine targets remain open.

Independent exact-source review accepted the four-path change; lifetime, caller-buffer aliasing, admission, codec/CRC and fallback coverage were reviewed. Automatic Codex review of exact final 67aee9e completed cleanly (review); no manual review request was needed. One hosted balanced-profile build stopped before benchmark execution on a Go dependency-proxy HTTP/2 download error. Its original log is preserved. One bounded failed-job refresh was spent on that transport failure; attempt 2 passed, and the other green jobs were retained. Remaining latest-head CI is still required before merge.

Reproduce the existing public benchmark with the pinned Go1.26.9 toolchain:

GOWORK=off GOMAXPROCS=2 go test -tags=treedb_test -run '^$' -bench '^BenchmarkNativeCollectionFeatureBatchCheckpoint$' -benchtime=8x -count=1 -benchmem ./TreeDB

Fixes #5181. Broader document-vector work remains #5171; full engine and Dgraph outcome gates remain #5139/#5145/#4879.

Summary by CodeRabbit

  • New Features

    • Added native physical leaf-page reads for captured database state and conditional transactions.
    • Native reads support caller-provided buffers while preserving data validity across checkpoints and database closure.
    • JSON leaf reads now use native physical data when available, with snapshot-based fallback support.
  • Reliability

    • Added safeguards for workspace capacity, closed scopes, transaction state, and destination-buffer admission.

Provisional integration of PR #5187 exact e23; no engine production changes.
The four selected native-reader blobs remain identical to 48bec4e.
Publication and merge wait for #5187 to land and final-base validation.
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c61b5e8e-ea5d-4d55-9754-1d78af1e138b
📥 Commits

Reviewing files that changed from the base of the PR and between 7c24acb and 67aee9e.

📒 Files selected for processing: 5
  • .github/ci/ci_impact.json
  • TreeDB/caching/cow_native_physical_read.go
  • TreeDB/caching/cow_native_physical_read_test.go
  • TreeDB/collections/native_json_read.go
  • TreeDB/db/native_semantic_state.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The change adds captured physical leaf-page reads to COW scopes. Native JSON reads use this path when supported and retain the snapshot-based fallback. The new test covers admission, decoding, buffer ownership, retained scopes, closed-state errors, and cleanup.

Changes: Captured native leaf reads

Layer / File(s) Summary
Reader contract and captured access
TreeDB/db/native_semantic_state.go, TreeDB/caching/cow_native_physical_read.go
The NativePhysicalLeafReader interface defines the captured leaf-read contract. COW basis, capture, and conditional-transaction methods enforce workspace limits and scope-state checks before delegating reads to the physical cache reader.
Native JSON routing and validation
TreeDB/collections/native_json_read.go, TreeDB/caching/cow_native_physical_read_test.go, .github/ci/ci_impact.json
readLeaf uses the physical reader when available and otherwise keeps the snapshot fallback. Tests cover admission, compressed-page decoding, caller-owned buffers, retained scopes, closed-state errors, and cleanup. The CI discovery hash is updated.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 67aee

Native JSON reads on retained captures now reuse the captured physical leaf workspace and fall back to the snapshot reader when that path is unavailable. Supported leaf codecs are unchanged, and closed captures still fail cleanly. No merge-blocking risk was identified beyond the pending CI and benchmark items noted in the PR.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Linked Issues check Inconclusive The implementation addresses the coding objectives in [#5181]. NativePhysicalLeafReader.NativeLeafPage provides caller-destination leaf decoding, and NativeJSONRead.readLeaf uses it with the exist… Provide the exact-head hosted caching/DB closure results, or equivalent evidence that those required automated checks passed for 67aee9e03d437eb29bc13535520c866a9060eec7.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: reusing captured physical leaf workspace for native collection reads.
Out of Scope Changes check Passed The reported changes stay within [#5181]. They add the optional physical leaf-reader interface, its capture and conditional-transaction implementations, the native JSON integration, and focused tests.…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (1 skipped: 1 …
Full details: Out of Scope Changes check

Explanation

The reported changes stay within [#5181]. They add the optional physical leaf-reader interface, its capture and conditional-transaction implementations, the native JSON integration, and focused tests. The only additional change refreshes the generated CI discovery checksum, which [#5181] explicitly permits. The PR reports no new owner, pool, cache, codec, format, protocol, document-vector, kind105, DATA-retirement, or SYSTEM-pager work.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (1 skipped: 1 unsupported.)

Full details: Linked Issues check

Explanation

The implementation addresses the coding objectives in [#5181]. NativePhysicalLeafReader.NativeLeafPage provides caller-destination leaf decoding, and NativeJSONRead.readLeaf uses it with the existing fallback. The capture path retains the basis lock, admitted workspace, physical decoder, and validation path. The added lifetime, admission, aliasing, close, and race tests are reported as passing. The reported full test tree matches the reviewed head, but exact-head hosted caching/DB closure remains pending. This leaves the required exact-head automated validation unconfirmed.

Resolution

Provide the exact-head hosted caching/DB closure results, or equivalent evidence that those required automated checks passed for 67aee9e03d437eb29bc13535520c866a9060eec7.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

PR #5187 landed as 7c24acb after frozen 695f6d integrated exact e23a723.
Main has the byte-identical e23a723 source tree already present in 695f6d.
This ancestry merge therefore retains the existing c54bd7b tree.
Preserve in-flight 695f6d local qualification; hosted CI certifies final head.
No production, test, policy or inventory changes.
@snissn
snissn marked this pull request as ready for review October 20, 2026 19:11
Copilot AI balanced review requested due to automatic review settings October 20, 2026 19:11
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-20T19:15:23.871251Z 67aee9e Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

collections: reuse captured leaf workspace on existing native public reads

2 participants