Skip to content

valuelog: split sealed vs current mmap telemetry#828

Open
snissn wants to merge 5 commits intopr/batch-arena-tail-compactionfrom
pr/vlog-mmap-telemetry-split
Open

valuelog: split sealed vs current mmap telemetry#828
snissn wants to merge 5 commits intopr/batch-arena-tail-compactionfrom
pr/vlog-mmap-telemetry-split

Conversation

@snissn
Copy link
Copy Markdown
Owner

@snissn snissn commented Mar 15, 2026

Summary

  • split value-log mmap residency stats into current-writable vs sealed mapped segments/bytes
  • export sealed-map-denied counters so Celestia runs can show when the sealed lazy-map budget forces ReadAt fallback
  • extend residency tests to assert the split accounting and denied-count aggregation

Why

  • the new mmap policy cut file-backed RSS materially, but the next iteration needs cleaner runtime evidence
  • separating current-vs-sealed residency tells us whether remaining mapped bytes are from the one hot writable segment or from sealed-segment reuse
  • sealed-map-denied counts make the fallback behavior explicit instead of inferred from Pss_File

Validation

  • go test ./TreeDB/internal/valuelog -run 'MmapResidencyStats|SealedLazyMmapBudget|PromoteCurrentWritable'
  • go test ./TreeDB/db ./TreeDB/caching
  • go vet ./TreeDB/internal/valuelog ./TreeDB/db ./TreeDB/caching

Stack

Copilot AI review requested due to automatic review settings March 15, 2026 08:35
Copy link
Copy Markdown
Contributor

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

This PR splits the value-log mmap residency telemetry into current-writable vs sealed-segment categories and exports a new sealed-map-denied counter. This builds on the mmap budgeting work in #827 to provide finer-grained runtime observability of which mapped bytes come from the hot writable segment versus sealed-segment reuse.

Changes:

  • Split MmapResidencyStats return values from 4 to 6, separating current-writable and sealed segment/byte counts, while preserving backward-compatible aggregate active_* stats keys.
  • Added sealedMapDeniedCount atomic counter on File and new SealedMapDeniedStats() aggregation on Manager, exported in both db and caching stats maps.
  • Updated the residency test to validate the split accounting and denied-count aggregation.

Reviewed changes

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

Show a summary per file
File Description
TreeDB/internal/valuelog/manager.go Split MmapResidencyStats return values; added sealedMapDeniedCount field and SealedMapDeniedStats() method
TreeDB/internal/valuelog/reader_mmap.go Increment sealedMapDeniedCount when sealed lazy mmap is denied
TreeDB/internal/valuelog/manager_test.go Updated test to assert split current/sealed accounting and denied stats
TreeDB/db/api.go Export new split and denied stats keys in DB.Stats()
TreeDB/caching/db.go Export new split and denied stats keys in caching DB.Stats()

💡 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.

Copy link
Copy Markdown
Contributor

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

This PR refines value-log mmap telemetry to better distinguish mapped residency sources and to make sealed-segment mmap budget behavior observable in production runs.

Changes:

  • Split Manager.MmapResidencyStats() into current-writable vs sealed mapped segments/bytes, and propagate the new stats into both backend (TreeDB/db) and cached (TreeDB/caching) Stats() outputs.
  • Track and export a sealed lazy-mmap “denied” counter (budget exhaustion) aggregated across files.
  • Update valuelog manager tests to assert split residency accounting and denied-count aggregation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
TreeDB/internal/valuelog/reader_mmap.go Increment per-file denied counter when sealed lazy-mmap is disallowed.
TreeDB/internal/valuelog/manager.go Add denied counter field, split residency stats into current vs sealed, and add an aggregate denied-stats method.
TreeDB/internal/valuelog/manager_test.go Extend residency aggregation test for the split counters and denied aggregation.
TreeDB/db/api.go Export new split residency stats and sealed-map-denied counter in backend DB Stats().
TreeDB/caching/db.go Export new split residency stats and sealed-map-denied counter in cached DB Stats().

💡 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.

Copilot AI review requested due to automatic review settings March 17, 2026 18:42
Copy link
Copy Markdown
Contributor

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

This PR enhances value-log mmap telemetry by splitting residency accounting into current-writable vs sealed segments, and by exposing counters for when sealed lazy-mapping is denied (forcing ReadAt fallback).

Changes:

  • Split MmapResidencyStats into current vs sealed segments/bytes and plumb new metrics into DB stats outputs.
  • Track and aggregate sealedMapDeniedCount across files and export it via SealedMapDeniedStats.
  • Extend unit tests to validate the split residency accounting and denied-count aggregation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
TreeDB/internal/valuelog/reader_mmap.go Increment per-file counter when sealed lazy-mmap is denied.
TreeDB/internal/valuelog/manager.go Split mmap residency stats (current vs sealed) and add aggregated denied-count stats.
TreeDB/internal/valuelog/manager_test.go Update tests to assert split residency accounting and denied-count aggregation.
TreeDB/db/api.go Export the new split metrics and denied counter in DB stats map.
TreeDB/caching/db.go Export the new split metrics and denied counter in caching DB stats map.

💡 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.

Copilot AI review requested due to automatic review settings March 18, 2026 00:24
Copy link
Copy Markdown
Contributor

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

Splits value-log mmap residency telemetry into current-writable vs sealed segments/bytes and exposes a counter for when sealed lazy-mmap is denied, enabling clearer runtime evidence of mmap policy behavior.

Changes:

  • Split MmapResidencyStats into current vs sealed segment/byte aggregates and update Stats exporters accordingly.
  • Track and export sealedMapDeniedCount so runs can observe when sealed lazy-mmap is denied.
  • Extend residency tests to assert the new split accounting and denied-count aggregation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
TreeDB/internal/valuelog/reader_mmap.go Records per-file denied counts when sealed lazy-mmap is not allowed.
TreeDB/internal/valuelog/manager.go Splits mmap residency stats and adds aggregation for sealed-map-denied counts.
TreeDB/internal/valuelog/manager_test.go Updates/extends tests to validate new split stats and denied aggregation.
TreeDB/db/api.go Exports new split residency stats and the denied counter in DB Stats.
TreeDB/caching/db.go Mirrors DB Stats export changes for the caching DB wrapper.

💡 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.

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.

3 participants