Skip to content
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

Jump to bottom

Add storage.wal.fsync.latency and other metrics #19425

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Add storage.wal.fsync.latency and other metrics #19425

wants to merge 10 commits into from
+24 −0

Conversation

rmloveland
Copy link
Contributor

Fixes DOC-11996

@rmloveland rmloveland marked this pull request as draft March 6, 2025 16:43
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 6, 2025 •
edited
Loading

Files changed:

@netlify Netlify
Copy link

netlify bot commented Mar 6, 2025 •
edited
Loading

✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit 21feb5c
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-interactivetutorials-docs/deploys/67f9373a9979540008a47cf1

@netlify Netlify
Copy link

netlify bot commented Mar 6, 2025 •
edited
Loading

✅ Deploy Preview for cockroachdb-api-docs canceled.

Name Link
🔨 Latest commit 21feb5c
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-api-docs/deploys/67f9373a08b5ae0008c35300

@netlify Netlify
Copy link

netlify bot commented Mar 6, 2025 •
edited
Loading

❌ Deploy Preview for cockroachdb-docs failed. Why did it fail? →

Name Link
🔨 Latest commit 21feb5c
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/67f9373aa5b5ad00086a3b5a

@rmloveland rmloveland force-pushed the 20250306-DOC-11996-storage-essential-metrics branch from 2392785 to 5d8469e Compare March 10, 2025 15:16
@rmloveland rmloveland changed the title Add storage.wal.fsync.latency to metrics Add storage.wal.fsync.latency and other metrics Mar 10, 2025
@rmloveland
Add storage.wal.fsync.latency and other metrics
78c6838 
Fixes DOC-11996

Adds the following metrics to the docs:

- storage.wal.fsync.latency
- rebalancing.range.rebalances
- rebalancing.replicas.queriespersecond
@rmloveland rmloveland force-pushed the 20250306-DOC-11996-storage-essential-metrics branch from 5d8469e to 78c6838 Compare March 10, 2025 15:32
@rmloveland rmloveland marked this pull request as ready for review March 13, 2025 18:11
@rmloveland rmloveland requested a review from kvoli March 13, 2025 18:11
@rmloveland rmloveland requested a review from kvoli March 18, 2025 15:09
jbowens
jbowens approved these changes Apr 8, 2025
View reviewed changes
Copy link

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

lgtm

seconding @sumeerbhola's slack comment that maybe we should update the WAL failover page to reference storage.wal.failover.write_and_sync.latency. The tl;dr is that storage.wal.fsync.latency monitors the latencies of WAL files. If you have WAL failover enabled and are failing over, storage.wal.fsync.latency will include the latency of the stalled primary. The storage.wal.failover.write_and_sync.latency metric is up one level, and during the failover will report the latency actually observed by higher-levels (which should be ~equivalent to the latency of the secondary).

@rmloveland
Copy link
Contributor Author

rmloveland commented Apr 10, 2025 •
edited
Loading

thanks for the review @jbowens!

and thanks for the additional info, i've filed https://cockroachlabs.atlassian.net/browse/DOC-13184 to update the WAL failover page with that metric as well

update: as part of that work i'll also plan to come back and update this list of metrics as well if we decide that's appropriate

@rmloveland rmloveland requested a review from taroface April 10, 2025 20:56
taroface
taroface approved these changes Apr 10, 2025
View reviewed changes
Copy link
Contributor

@taroface taroface left a comment

Choose a reason for hiding this comment

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

LGTM. Just a few small suggestions.

@rmloveland rmloveland enabled auto-merge (squash) April 11, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbowens jbowens jbowens approved these changes

@taroface taroface taroface approved these changes

@kvoli kvoli kvoli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rmloveland @jbowens @taroface @kvoli