-
Notifications
You must be signed in to change notification settings - Fork 887
Add hashLogger for per module hash and metrics for per module stats #3778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e737ff5
140f8e8
08419db
c35f5f0
11088bb
6dd8e3c
9d353bb
7097a11
affaffb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,7 @@ func (s *CommitStore) Commit() (version int64, err error) { | |
|
|
||
| s.phaseTimer.SetPhase("commit_done") | ||
| otelMetrics.CurrentVersion.Record(s.ctx, version) | ||
| s.recordAllModuleStats() | ||
| logger.Info("FlatKV Commit complete", | ||
| "version", version, | ||
| "totalWriteCount", pendingAccount+pendingCode+pendingStorage+pendingMisc, | ||
|
|
@@ -225,7 +226,12 @@ func (s *CommitStore) commitBatches(version int64) error { | |
| } | ||
| } | ||
|
|
||
| // Update in-memory local meta after all commits succeed. | ||
| // Update in-memory local meta after all commits succeed. Per-module stats gauges are deliberately | ||
| // NOT recorded here: commitBatches is also called by catchup (WAL replay on open, import, rollback, | ||
| // and openReadOnly's historical read-only clone), and recording from every replayed entry would | ||
| // publish stale/historical values into the process-global gauges, clobbering the live store's real | ||
| // values. Only Commit's success path (an actual new block being committed) records via | ||
| // recordAllModuleStats. | ||
| for _, p := range pending { | ||
| s.localMeta[p.dbDir] = &ktype.LocalMeta{ | ||
| CommittedVersion: version, | ||
|
|
@@ -237,6 +243,16 @@ func (s *CommitStore) commitBatches(version int64) error { | |
| return nil | ||
| } | ||
|
|
||
| // recordAllModuleStats records the current per-module key-count / byte totals for every data DB. Called | ||
| // only from Commit's success path — i.e. only for an actual new block being committed, never for open | ||
| // (LoadVersion/catchup), import (FinalizeImport), rollback, or a read-only historical replay — so the | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] The exclusion of catchup/WAL-replay here is well reasoned (replaying historical entries would clobber the live gauges). But |
||
| // process-global gauges always reflect the live store, not a replay. | ||
| func (s *CommitStore) recordAllModuleStats() { | ||
| for _, dir := range dataDBDirs { | ||
| recordModuleStats(s.ctx, dir, s.perDBModuleWorkingStats[dir]) | ||
| } | ||
| } | ||
|
Comment on lines
+246
to
+254
This comment was marked as low quality.
Sorry, something went wrong.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fine There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Module gauges skipped on openMedium Severity
Reviewed by Cursor Bugbot for commit affaffb. Configure here. |
||
|
|
||
| func prepareBatch[T vtype.VType]( | ||
| db types.KeyValueDB, | ||
| writes map[string]T, | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] Nit: the capacity hint
len(dataDBDirs)+1no longer accounts for the per-module categories appended in the loop, so this slice will reallocate as soon as any module is present. Harmless, but consider dropping the hint or sizing it with the module count if you want to keep the pre-allocation meaningful.