caching: direct-arena telemetry + oldest-eviction retention fix#837
caching: direct-arena telemetry + oldest-eviction retention fix#837snissn wants to merge 5 commits intopr/append-only-lease-trim-pool-reusefrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0172726d5
ℹ️ 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
Adds visibility and correct retention behavior for the append-only direct value arena used by TreeDB caching, and wires the new counters into unified bench output + tests.
Changes:
- Added process-wide telemetry counters for append-only direct arena reuse vs fresh allocations (chunks/bytes).
- Changed retained-chunk eviction policy to evict the oldest retained chunks first (preserving the newest working set).
- Added a deterministic unit test to lock in “newest-window” retention behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cmd/unified_bench/main.go | Includes new append-only direct arena counters in the “small/stable” cache stats output list. |
| TreeDB/caching/db.go | Implements telemetry counters, exposes them via DB.Stats(), and updates retention eviction to FIFO/oldest-first. |
| TreeDB/caching/append_only_direct_writer_pool_test.go | Adds unit coverage for oldest-first eviction / newest-window retention behavior. |
💡 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.
Pull request overview
This PR enhances observability and retention behavior for the append-only direct value arena used in TreeDB/caching, adding counters for reuse/allocation and adjusting retention eviction to prefer keeping newly produced chunks.
Changes:
- Add global counters and
DB.Stats()exports for append-only direct arena pool hits, retained hits, and fresh allocations (chunks/bytes). - Change
appendOnlyDirectValueArena.retainChunkseviction policy to evict older retained chunks first (to keep newer chunks). - Extend test coverage and benchmark stat printing to include the new append-only direct arena stats.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| TreeDB/caching/db.go | Adds append-only arena counters + stats export; changes retained-eviction logic to evict from the “oldest” end. |
| TreeDB/caching/append_only_direct_writer_pool_test.go | Adds a test intended to verify “evict oldest / keep newest” retention behavior. |
| cmd/unified_bench/main.go | Prints the newly added append-only direct arena stats in the unified benchmark output. |
💡 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.
…ena-retention-telemetry-oldest-eviction
…ena-retention-telemetry-oldest-eviction
…ena-retention-telemetry-oldest-eviction
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 848e450d12
ℹ️ 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
Adds telemetry for the append-only direct arena (pool hits, retained hits, fresh allocations) and fixes the retention eviction order to drop the oldest retained chunks first, preserving the newest working set under pressure.
Changes:
- Added direct-arena allocation/reuse counters to
DB.Stats(). - Exposed the new counters in
unified-bench -treedb-cache-stats-after-testsoutput. - Changed retention eviction to evict oldest-first and added a deterministic unit test for newest-window retention behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cmd/unified_bench/main.go | Adds new append-only direct arena stat keys to the unified bench cache stats output list. |
| TreeDB/caching/db.go | Implements new atomic telemetry counters; changes retained-chunk eviction to oldest-first; exports counters via DB.Stats(). |
| TreeDB/caching/append_only_direct_writer_pool_test.go | Adds a unit test asserting oldest chunks are evicted and newest are retained under capacity pressure. |
💡 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.
…ena-retention-telemetry-oldest-eviction
Summary
DB.Stats()for pool hits, retained hits, and fresh allocations (chunks/bytes)unified-bench -treedb-cache-stats-after-testsoutputWhy
Evidence
Unified bench command used (full suite):
Baseline log/profile:
/tmp/unified-run-tele4.log,/home/mikers/tmp/perf-direct-telemetry-1773589060This PR log/profile:
/tmp/unified-run-evict-oldest-rebuilt.log,/tmp/optimize-reads-1773589496Key deltas from post-
prefix_scancounters:fresh_alloc_chunks_total:2650 -> 2645(-0.19%)fresh_alloc_bytes_total:86835200 -> 86671360(-0.19%)retained_hit_chunks_total:3503 -> 3511(+0.23%)retained_hit_bytes_total:114786304 -> 115048448(+0.23%)Selected throughput deltas (single-run, same command):
4,156,111 -> 4,383,749(+5.48%)5,251,396 -> 5,314,813(+1.21%)12,524,669 -> 12,660,427(+1.08%)2,140,181 -> 2,181,933(+1.95%)Random-write alloc-space hotspot (
getAppendOnlyDirectValueArenaChunk):20598.50kB -> 15845kB(single-run snapshot)Validation
go test ./TreeDB/caching -count=1go test ./cmd/unified_bench -count=1go vet ./TreeDB/caching ./cmd/unified_bench