-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
kvserver: add storage.sstable.zombie.bytes metric #122152
Conversation
Add a new timeseries metric that provides visibility into the volume of data that exists in sstables that are not part of the most recent version of the LSM. Epic: none Informs cockroachdb#121935. Informs cockroachdb#122139. Informs cockroachdb/pebble#3500. Close cockroachdb#122110. Release note (ops change): Adds a new timeseries metric storage.sstable.zombie.bytes.
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
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.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @jbowens)
pkg/kv/kvserver/metrics.go
line 3736 at r1 (raw file):
sm.BatchCommitWALRotWaitDuration.Update(int64(m.BatchCommitStats.WALRotationDuration)) sm.BatchCommitCommitWaitDuration.Update(int64(m.BatchCommitStats.CommitWaitDuration)) sm.SSTableZombieBytes.Update(int64(m.Table.ZombieSize))
.Table.Local.ZombieSize
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.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @RaduBerinde)
pkg/kv/kvserver/metrics.go
line 3736 at r1 (raw file):
Previously, RaduBerinde wrote…
.Table.Local.ZombieSize
I debated this and thought we'd prefer the wider, more all-encompassing one. Even with disaggregated storage, we may care about zombie sstables in blob storage.
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.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @itsbilal)
pkg/kv/kvserver/metrics.go
line 3736 at r1 (raw file):
Previously, jbowens (Jackson Owens) wrote…
I debated this and thought we'd prefer the wider, more all-encompassing one. Even with disaggregated storage, we may care about zombie sstables in blob storage.
👍
TFTR! bors r=RaduBerinde |
Add a new timeseries metric that provides visibility into the volume of data that exists in sstables that are not part of the most recent version of the LSM.
Epic: none
Informs #121935.
Informs #122139.
Informs cockroachdb/pebble#3500.
Close #122110.
Release note (ops change): Adds a new timeseries metric storage.sstable.zombie.bytes.