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.

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

mvcc: add metrics to characterize backend expensive reads #1

Conversation

jingyih
Copy link

@jingyih jingyih commented Feb 12, 2019

Metrics added:

etcd_debugging_mvcc_put_total
etcd_debugging_mvcc_put_duration_seconds
etcd_debugging_mvcc_range_total
etcd_debugging_mvcc_range_duration_seconds
etcd_debugging_mvcc_committed_read_total
etcd_debugging_mvcc_committed_read_duration_seconds
etcd_debugging_backend_batch_interval_duration_seconds

@jingyih jingyih force-pushed the expensive_read_instrumentation branch 2 times, most recently from 676d2dd to 324330c Compare February 12, 2019 06:52
@@ -315,6 +316,8 @@ func (b *backend) run() {
if b.batchTx.safePending() != 0 {
b.batchTx.Commit()
}
batchIntervalSec.Observe(time.Since(start).Seconds() * 1e3)
Copy link
Owner

Choose a reason for hiding this comment

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

I believe .Seconds() already performs the needed unit conversion and we don't need the * 1e3.

Copy link
Author

Choose a reason for hiding this comment

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

Switched to seconds.

batchIntervalSec = prometheus.NewHistogram(prometheus.HistogramOpts{
Namespace: "etcd_debugging",
Subsystem: "backend",
Name: "batch_interval_duration_milliseconds",
Copy link
Owner

Choose a reason for hiding this comment

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

variable name suggests seconds but metric name suggests milliseconds. For prometheus it's okay to use seconds here. There's a convention: https://prometheus.io/docs/practices/naming/#base-units


// lowest bucket start of upper bound 0.1 msec (10 us) with factor 2
// highest bucket start of 0.1 msec * 2^13 == 819.2 msec
Buckets: prometheus.ExponentialBuckets(0.1, 2, 14),
Copy link
Owner

Choose a reason for hiding this comment

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

If we switch to seconds, we'll need to update the buckets to match.

@jingyih jingyih force-pushed the expensive_read_instrumentation branch from 324330c to 51e6dce Compare February 12, 2019 20:55
@jpbetz jpbetz merged commit ba48da9 into jpbetz:expensive_read_instrumentation Feb 12, 2019
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.

2 participants