Skip to content

fix(seidb-metrics): use shared fine-grained buckets for memiavl/flatkv/mvcc histograms #3771

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.

Sign up for GitHub

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

Open
blindchaser wants to merge 1 commit into
base: main
Choose a base branch
from
+35 −0
Open

fix(seidb-metrics): use shared fine-grained buckets for memiavl/flatkv/mvcc histograms #3771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions sei-db/db_engine/pebbledb/mvcc/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import (
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/metric"

smetrics "github.com/sei-protocol/sei-chain/sei-db/common/metrics"
)

var (
Expand Down Expand Up @@ -42,31 +44,37 @@
"pebble_get_latency",
metric.WithDescription("Time taken to get a key from PebbleDB"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
applyChangesetLatency: must(meter.Float64Histogram(
"pebble_apply_changeset_latency",
metric.WithDescription("Time taken to apply changeset to PebbleDB"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
applyChangesetAsyncLatency: must(meter.Float64Histogram(
"pebble_apply_changeset_async_latency",
metric.WithDescription("Time taken to queue changeset for async write"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
pruneLatency: must(meter.Float64Histogram(
"pebble_prune_latency",
metric.WithDescription("Time taken to prune old versions from PebbleDB"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
importLatency: must(meter.Float64Histogram(
"pebble_import_latency",
metric.WithDescription("Time taken to import snapshot data to PebbleDB"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
batchWriteLatency: must(meter.Float64Histogram(
"pebble_batch_write_latency",
metric.WithDescription("Time taken to write a batch to PebbleDB"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),

compactionCount: must(meter.Int64Counter(
Expand All @@ -78,6 +86,7 @@
"pebble_compaction_duration",
metric.WithDescription("Duration of compaction operations"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
compactionBytesRead: must(meter.Int64Counter(
"pebble_compaction_bytes_read",
Expand All @@ -99,6 +108,7 @@
"pebble_flush_duration",
metric.WithDescription("Duration of memtable flush operations"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
flushBytesWritten: must(meter.Int64Counter(
"pebble_flush_bytes_written",
Expand Down Expand Up @@ -149,12 +159,13 @@
)),

batchSize: must(meter.Int64Histogram(
"pebble_batch_size",
metric.WithDescription("Size of batches written to PebbleDB"),
metric.WithUnit("By"),
metric.WithExplicitBucketBoundaries(smetrics.ByteSizeBuckets...),

@seidroid seidroid Bot Jul 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] pebble_batch_size records int64(len(ops)) (an operation count) at batch.go:170, not bytes, yet this assigns ByteSizeBuckets (256B–1GB). Batches almost always have far fewer than 256 ops, so every sample falls in the lowest bucket — reproducing the exact p95/p99 resolution failure this PR aims to fix. The metric's unit is also "By" while it records a count. Either use smetrics.CountBuckets to match the current recorded value, or record the actual batch byte size (e.g. batch.Len()) to match the By unit and the "Size of batches" description.

)),
pendingChangesQueueDepth: must(meter.Int64Gauge(
"pebble_pending_changes_queue_depth",

Check warning on line 168 in sei-db/db_engine/pebbledb/mvcc/metrics.go

View check run for this annotation

Claude / Claude Code Review

pebble_batch_size histogram uses ByteSizeBuckets but records an operation count 

This PR adds `smetrics.ByteSizeBuckets` (256B–1GB) to the `pebble_batch_size` histogram, but the only recording site (`writeBatchOps` in batch.go) records `batchSize := int64(len(ops))` — a count of batch operations, not a byte size. Since op counts (tens–thousands) fall far below the first ByteSizeBuckets boundary of 256, nearly every sample will land in the lowest bucket, reproducing the exact histogram_quantile resolution problem this PR is fixing elsewhere; it should use `smetrics.CountBucke
Comment on lines 162 to 168

@claude claude Bot Jul 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 This PR adds smetrics.ByteSizeBuckets (256B–1GB) to the pebble_batch_size histogram, but the only recording site (writeBatchOps in batch.go) records batchSize := int64(len(ops)) — a count of batch operations, not a byte size. Since op counts (tens–thousands) fall far below the first ByteSizeBuckets boundary of 256, nearly every sample will land in the lowest bucket, reproducing the exact histogram_quantile resolution problem this PR is fixing elsewhere; it should use smetrics.CountBuckets instead, matching the analogous iteratorIterations metric two lines below.

Extended reasoning...

The bug: metrics.go:165 applies metric.WithExplicitBucketBoundaries(smetrics.ByteSizeBuckets...) to the pebble_batch_size histogram (otelMetrics.batchSize, an Int64Histogram). ByteSizeBuckets (defined in sei-db/common/metrics/buckets.go:17-20) is [256, 1KB, 4KB, 16KB, 64KB, 256KB, 1MB, 4MB, 16MB, 64MB, 256MB, 1GB] — designed for byte-size data.

Where it's recorded: The only call site is writeBatchOps in sei-db/db_engine/pebbledb/mvcc/batch.go:169-178:

batchSize := int64(len(ops))
...
otelMetrics.batchSize.Record(ctx, batchSize)

ops is the []batchOp slice passed in — one entry per Set/Delete call being written in that pebble batch. So the value recorded is an operation count, not a byte size, despite the metric's unit (By) and description ('Size of batches written to PebbleDB') suggesting otherwise. That unit/name mismatch pre-dates this PR, but the bucket-boundary choice is new in this diff and is what actually determines whether the histogram is usable.

Why this defeats the PR's own goal: Real-world batch op counts per commit are typically in the tens-to-low-thousands range — well below the first ByteSizeBuckets boundary of 256. That means nearly all samples fall into the le="256" bucket, and histogram_quantile cannot resolve p95/p99 for this metric — precisely the 'everything piles into one bucket' failure mode this PR's description says it's fixing for the other histograms.

Step-by-step proof:

  1. A block commits a changeset with, say, 40 Set/Delete ops → writeBatchOps is called with len(ops) == 40.
  2. batchSize := int64(len(ops)) → batchSize = 40.
  3. otelMetrics.batchSize.Record(ctx, 40) records the value 40 into a histogram bucketed at [256, 1024, 4096, ...].
  4. 40 < 256, so this sample (and virtually every other realistic sample, since op counts rarely exceed a few thousand) lands in the le="256" bucket.
  5. Querying histogram_quantile(0.95, rate(pebble_batch_size_bucket[5m])) returns ≈256 (or whatever the lowest boundary is) regardless of the actual distribution of batch sizes, because all the mass is in one bucket — no resolution.

The fix: Use smetrics.CountBuckets ([1, 5, 10, 50, 100, 500, 1000, 5000, 10000, 100000, 1000000]) instead of ByteSizeBuckets for this histogram, matching the pattern already correctly applied two lines below to iteratorIterations — a genuinely analogous per-operation count metric that uses CountBuckets.

Severity: This is an observability/dashboard-resolution issue only — no functional/runtime impact, no crash, no data loss. It just means this one specific histogram (pebble_batch_size) won't have useful percentiles, which is a quality gap relative to the PR's stated intent but doesn't block any behavior. Marking as nit.

metric.WithDescription("Number of pending changesets in async write queue"),
metric.WithUnit("{count}"),
)),
Expand All @@ -162,6 +173,7 @@
"pebble_iterator_iterations",
metric.WithDescription("Number of iterations per iterator"),
metric.WithUnit("{count}"),
metric.WithExplicitBucketBoundaries(smetrics.CountBuckets...),
)),
}
)
Expand Down
14 changes: 14 additions & 0 deletions sei-db/state_db/sc/flatkv/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"

smetrics "github.com/sei-protocol/sei-chain/sei-db/common/metrics"
)

var (
Expand Down Expand Up @@ -38,26 +40,31 @@ var (
"flatkv_open_latency",
metric.WithDescription("Time taken to open the FlatKV store"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
ApplyChangesetsLatency: must(flatkvMeter.Float64Histogram(
"flatkv_apply_changesets_latency",
metric.WithDescription("Time taken to apply changesets to FlatKV"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
CommitLatency: must(flatkvMeter.Float64Histogram(
"flatkv_commit_latency",
metric.WithDescription("Time taken to commit FlatKV changes"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
CommitBatchLatency: must(flatkvMeter.Float64Histogram(
"flatkv_commit_batch_latency",
metric.WithDescription("Time taken to commit a FlatKV data DB batch"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
BatchReadOldValuesLatency: must(flatkvMeter.Float64Histogram(
"flatkv_batch_read_old_values_latency",
metric.WithDescription("Time taken to batch read old FlatKV values"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
NumKVPairs: must(flatkvMeter.Int64Counter(
"flatkv_num_kv_pairs",
Expand All @@ -78,6 +85,7 @@ var (
"flatkv_catchup_latency",
metric.WithDescription("Time taken to replay FlatKV WAL entries"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
CatchupReplayNumBlocks: must(flatkvMeter.Int64Counter(
"flatkv_catchup_replay_num_blocks",
Expand All @@ -88,11 +96,13 @@ var (
"flatkv_snapshot_write_latency",
metric.WithDescription("Time taken to write a FlatKV snapshot"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
SnapshotPruneLatency: must(flatkvMeter.Float64Histogram(
"flatkv_snapshot_prune_latency",
metric.WithDescription("Time taken to prune FlatKV snapshots"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
SnapshotPruneAttempts: must(flatkvMeter.Int64Counter(
"flatkv_snapshot_prune_attempts",
Expand All @@ -108,11 +118,13 @@ var (
"flatkv_rollback_latency",
metric.WithDescription("Time taken to rollback FlatKV state"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
ImportLatency: must(flatkvMeter.Float64Histogram(
"flatkv_import_latency",
metric.WithDescription("Time taken to import FlatKV snapshot data"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use longer buckets for snapshot imports

For state-sync imports that run longer than 5 minutes, smetrics.LatencyBuckets tops out at 300s (sei-db/common/metrics/buckets.go), so every multi-minute/hour import lands only in the +Inf bucket and histogram_quantile cannot distinguish a 6-minute import from a multi-hour one. The migration docs explicitly note snapshot export/import can take hours when state grows (docs/migration/seidb_migration.md), so flatkv_import_latency should keep longer/default boundaries or use a separate long-operation bucket set while the per-block latencies use the fine-grained buckets.

Useful? React with 👍 / 👎.

)),
ImportKVPairs: must(flatkvMeter.Int64Counter(
"flatkv_import_kv_pairs",
Expand All @@ -123,11 +135,13 @@ var (
"flatkv_import_worker_flush_latency",
metric.WithDescription("Time taken to flush a FlatKV import worker batch"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
FlushLatency: must(flatkvMeter.Float64Histogram(
"flatkv_flush_latency",
metric.WithDescription("Time taken to flush a FlatKV data DB"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
}
)
Expand Down
9 changes: 9 additions & 0 deletions sei-db/state_db/sc/memiavl/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package memiavl
import (
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/metric"

smetrics "github.com/sei-protocol/sei-chain/sei-db/common/metrics"
)

var (
Expand All @@ -28,11 +30,13 @@ var (
"memiavl_restart_latency",
metric.WithDescription("Time taken to restart the memiavl database"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
SnapshotRewriteLatency: must(meter.Float64Histogram(
"memiavl_snapshot_rewrite_latency",
metric.WithDescription("Time taken to write to the new memiavl snapshot"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
NumSnapshotRewriteAttempts: must(meter.Int64Counter(
"memiavl_num_snapshot_rewrite_attempts",
Expand All @@ -42,6 +46,7 @@ var (
"memiavl_snapshot_prune_latency",
metric.WithDescription("Time taken to prune memiavl snapshot"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
NumSnapshotPruneAttempts: must(meter.Int64Counter(
"memiavl_num_snapshot_prune_attempts",
Expand All @@ -52,11 +57,13 @@ var (
"memiavl_snapshot_catchup_after_rewrite_latency",
metric.WithDescription("Time taken to catchup and replay after snapshot rewrite"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
CatchupBeforeReloadLatency: must(meter.Float64Histogram(
"memiavl_snapshot_catchup_before_reload_latency",
metric.WithDescription("Time taken to catchup and replay before switch to new snapshot"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
CatchupReplayNumBlocks: must(meter.Int64Counter(
"memiavl_snapshot_catchup_replay_num_blocks",
Expand All @@ -71,11 +78,13 @@ var (
"memiavl_commit_latency",
metric.WithDescription("Time taken to commit"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
ApplyChangesetLatency: must(meter.Float64Histogram(
"memiavl_apply_changeset_latency",
metric.WithDescription("Time taken to apply changesets"),
metric.WithUnit("s"),
metric.WithExplicitBucketBoundaries(smetrics.LatencyBuckets...),
)),
NumOfKVPairs: must(meter.Int64Counter(
"memiavl_num_of_kv_pairs",
Expand Down
Loading