valuelog: disable current writable mmaps by default#848
Conversation
There was a problem hiding this comment.
Pull request overview
Disables persistent mmap growth for current-writable value-log segments by default (opt-in via env var), while improving observability for RSS and value-log mmap residency (including multi-DB expvar attribution) and tightening correctness around current-segment read visibility and dict training.
Changes:
- Disable persistent current-writable value-log mmaps by default and add a current-writable read barrier to flush buffered tails before backend reads.
- Add process RSS/HWM + peak tracking and expose per-instance expvar stats to attribute memory usage in multi-DB processes.
- Expand Zstd dict autotune candidates / stabilize dict training offsets; add additional corruption/staleness regression tests.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/unified_bench/main.go | Include new process/mmap memory stats keys in benchmark stats output. |
| TreeDB/internal/valuelog/writer.go | Add PendingBytes() to report buffered-but-unflushed writer tail. |
| TreeDB/internal/valuelog/reader_mmap.go | Gate persistent mmaps for current-writable segments behind TREEDB_VLOG_ENABLE_CURRENT_WRITABLE_MMAP. |
| TreeDB/internal/valuelog/mmap_safety_test.go | Force-enable current-writable mmap for mmap safety regression tests. |
| TreeDB/internal/valuelog/manager_test.go | Add test ensuring current-writable reads fall back safely without persistent mmap. |
| TreeDB/internal/valuelog/manager.go | Add current-writable read barrier hook and invoke it on reads. |
| TreeDB/internal/valuelog/dict_invalid_offsets_test.go | Add regression coverage for invalid repeat-offset dictionaries from small histories. |
| TreeDB/internal/valuelog/dict_compressibility_bench_test.go | Use explicit non-zero repeat offsets to avoid invalid dictionaries in benches. |
| TreeDB/internal/valuelog/dict_bench_helper_test.go | Add tests asserting bench dict builders use safe initial offsets. |
| TreeDB/internal/valuelog/dict_autotune_bench_test.go | Use explicit offsets when building dicts with history in benches. |
| TreeDB/internal/valuelog/autotune_options_test.go | Update tests to validate expanded default candidate sets. |
| TreeDB/internal/valuelog/autotune_options.go | Expand default candidate history/dict sizes for autotune. |
| TreeDB/internal/memtable/hash_sorted_corruption_test.go | Add regression tests for stale-index recovery and rebuild correctness. |
| TreeDB/internal/memtable/hash_sorted.go | Harden read paths against stale indices; improve frozen-index validation. |
| TreeDB/internal/compression/trainer_test.go | Add coverage for selecting expanded history/dict candidate sizes. |
| TreeDB/internal/compression/trainer.go | Support separate history/dict candidates; shape/validate dict sizes; cap max history. |
| TreeDB/db/leaf_page_log.go | Promote registered segments to current-writable and expose a read-barrier setter. |
| TreeDB/cmd/vlog_dict_realdata/main_dict_test.go | Add harness test ensuring fixed-size trainer uses safe offsets. |
| TreeDB/cmd/vlog_dict_realdata/main.go | Plumb CandidateDictBytes and use explicit offsets for dict training. |
| TreeDB/caching/vlog_dict.go | Pass dict-size candidates into the compression trainer. |
| TreeDB/caching/vlog_current_segment_readbarrier_test.go | Add regression tests ensuring buffered tails are flushed before reads. |
| TreeDB/caching/vlog_autotune_bench.go | Update bench defaults for expanded history/dict candidate sizes. |
| TreeDB/caching/snapshot.go | Switch snapshot backend reads to use the new flushValueLogForBackendRead() barrier. |
| TreeDB/caching/process_memory_other.go | Stub RSS reader for non-Linux platforms. |
| TreeDB/caching/process_memory_linux.go | Implement RSS/HWM parsing via /proc/self/status on Linux. |
| TreeDB/caching/memory_stats_test.go | Add assertions for new process RSS/peak and backend mmap attribution stats. |
| TreeDB/caching/expvar_stats_test.go | Extend expvar selection tests and add multi-instance expvar payload coverage. |
| TreeDB/caching/expvar_stats.go | Track all open DB instances for expvar and include backend mmap/identity families. |
| TreeDB/caching/db.go | Add process memory sampler + peak stats, backend/cache mmap aggregation, and stronger backend-read flushing barrier. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fa4008cf8
ℹ️ 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
Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78c4a72e41
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 717c94c47a
ℹ️ 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".
|
Update: landed rewrite/dict optimization in What changed
Validation
Key numbers (post
Frame-level attribution (post-rewrite audits)
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 49 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc39d8afac
ℹ️ 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".
|
Compression A/B update: raised rewrite dict batch cap from k=32 to k=64 (commit 2af5db3), validated on same-input clones for both profiles.\n\nControlled same-input rewrite deltas:\n\n| profile | pre wal bytes | k32 wal bytes | k64 wal bytes | k64 vs k32 | k32 rewrite (s/rss_kb) | k64 rewrite (s/rss_kb) | k32 gzip ratio | k64 gzip ratio | k32 dict frames | k64 dict frames |\n|---|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|\n| fast | 61,226,647 | 38,993,579 | 38,420,980 | -572,599 (-1.47%) | 53.73 / 231,040 | 51.83 / 220,056 | 0.644889 | 0.658781 | 20,510 | 10,741 |\n| wal_on_fast | 124,995,002 | 38,978,062 | 38,445,903 | -532,159 (-1.37%) | 30.73 / 221,716 | 30.02 / 222,440 | 0.673229 | 0.688458 | 21,073 | 11,746 |\n\nNotes:\n- k64 consistently reduces post-rewrite on-disk bytes on identical input for both profiles.\n- rewrite wall is flat/slightly better; rewrite max_rss unchanged.\n- grouped_dict frame count drops ~48%, consistent with lower frame metadata overhead.\n\nArtifacts under /tmp/treedb_rewrite_ab_20260326062636 on bench host. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 50 out of 50 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
TreeDB/internal/memtable/hash_sorted.go:468
Getreturns the same tuple in both the pointer and non-pointer branches here, so theif ent.flags&node.FlagPointer != 0check is redundant. Consider returning directly once after unlocking to keep this path easier to read (and consistent withGetEntry, which has distinct pointer/non-pointer behavior).
if ent.flags&node.FlagPointer != 0 {
return ent.value, ent.flags&node.FlagTombstone != 0, true
}
return ent.value, ent.flags&node.FlagTombstone != 0, true
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47470e4ab2
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51a41f6cd9
ℹ️ 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
Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe789f9ea9
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1edf57edfc
ℹ️ 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
Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ab9c0787e
ℹ️ 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".
| valueLen := binary.LittleEndian.Uint32(header[16:20]) | ||
| out.rawRecords++ | ||
| out.rawPayloadBytes += int64(valueLen) | ||
| if valueLen > uint32(cap(payload)) { | ||
| payload = make([]byte, valueLen) |
There was a problem hiding this comment.
scanFrameMeta trusts the on-disk valueLen and uses it directly in make([]byte, valueLen). For corrupted or adversarial value-log input, a forged header can set valueLen near 4 GiB, causing wal_classify -frame-meta to attempt a huge allocation and OOM instead of reporting corruption. Add a maximum-length guard (for example limits.MaxRecordSize - valuelog.HeaderSize) before growing payload.
Useful? React with 👍 / 👎.
Summary
TREEDB_VLOG_ENABLE_CURRENT_WRITABLE_MMAP=1Why
fastprofiling showed the real RSS regression was not just heap pressure. Theapplication.dbTreeDB instance was accumulating large current-writable value-log remap state, with dead mmap bytes growing into the tens of GB during restore. Disabling persistent current-writable mmaps removes that churn and materially lowers peak RSS.Validation
GOWORK=off go test ./TreeDB/caching ./TreeDB/internal/valuelog -count=1GOWORK=off go vet ./TreeDB/caching ./TreeDB/internal/valuelogfastA/B:18.54M kB10.85M kB7.33 GiB/41.46%