caching: keep deeper append-only memtable lease reuse#818
caching: keep deeper append-only memtable lease reuse#818snissn wants to merge 2 commits intopr/appendonly-direct-arena-adaptive-retainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts append-only memtable recycling behavior in TreeDB/caching to retain more “warm” append-only memtables under strong references, reducing allocation churn during rotate/checkpoint-heavy write workloads.
Changes:
- Increase the bounded strong-reference lease queue for recycled append-only memtables (
maxAppendOnlyMemLeases) from 8 to 32. - Add a focused unit test to ensure
putAppendOnlyMemLeaserespects the configured cap.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| TreeDB/caching/db.go | Raises the cap on retained append-only memtable leases to improve reuse and reduce sync.Pool fallback under pressure. |
| TreeDB/caching/memtable_view_refs_test.go | Adds a regression test asserting the append-only memtable lease queue remains bounded by maxAppendOnlyMemLeases. |
💡 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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12a525e44e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adjusts append-only memtable lease retention in the caching layer to improve entry-slice reuse under rotation/checkpoint-heavy write workloads, and adds a unit test to prevent regressions in the lease cap behavior.
Changes:
- Increase
maxAppendOnlyMemLeasesfrom 8 to 32 with updated rationale in comments. - Add
TestPutAppendOnlyMemLease_RespectsCapto verify the lease list never grows beyond the configured cap.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
TreeDB/caching/db.go |
Raises the cap on strong references for recycled append-only memtables to reduce fallback to sync.Pool and allocation churn. |
TreeDB/caching/memtable_view_refs_test.go |
Adds a test ensuring putAppendOnlyMemLease enforces the maximum lease count. |
💡 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.
…endonly-entry-reuse-depth
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e9b61f4db
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Summary
maxAppendOnlyMemLeasesfrom8to32sync.PoolWhy
allocs_batch_write_steady_treedb.pprofshowedgetAppendOnlyEntriesFromPoolas a major allocator coming from memtable rotation/checkpoint paths. A shallow lease queue causes repeated entry-slice reallocation when rotate pressure is high.Validation
go test ./TreeDB/cachingtime ./bin/unified-bench -dbs treedb -profile fast -keys 500000 -progress=false -treedb-index-outer-leaves-in-vlog=true -valsize=100 -treedb-force-value-pointers=false -checkpoint-between-tests -profile-dir=/home/mikers/tmp/perf-entry-lease32b-1773434944Comparison baseline (PR #817 head):
/home/mikers/tmp/perf-direct-retainfix-1773434559alloc_space (batch_write_steady)
165.08MB -> 138.13MBgetAppendOnlyEntriesFromPool:26.72MB -> ~0MB(drops out of top with-nodefraction=0)ops/sec highlights
batch_write_steady:1,998,857 -> 2,107,601(+5.4%)batch_random:7,853,279 -> 8,377,934(+6.7%)dataset_write_random:4,845,198 -> 5,072,183(+4.7%)dataset_write_sorted:3,483,792 -> 3,656,106(+4.9%)sequential_write:4,116,846 -> 4,085,790(-0.8%, within noise)