-
Notifications
You must be signed in to change notification settings - Fork 548
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
block-builder: Add cortex_blockbuilder_blocks_produced_total metric #10538
Conversation
Signed-off-by: Ganesh Vernekar <[email protected]>
if cfg.NoPartiallyConsumedRegion { | ||
// We should not have a large buffer if we are putting all the records into a block. | ||
cfg.ConsumeIntervalBuffer = 5 * time.Minute | ||
} |
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.
By removing this override, we can now try different buffer times when we have set NoPartiallyConsumedRegion to true
Signed-off-by: Ganesh Vernekar <[email protected]>
prev, curr, next := getBlockCategoryCount(sectionEndTime, blockMetas) | ||
b.blockBuilderMetrics.blockCounts.WithLabelValues("previous").Add(float64(prev)) | ||
b.blockBuilderMetrics.blockCounts.WithLabelValues("current").Add(float64(curr)) | ||
b.blockBuilderMetrics.blockCounts.WithLabelValues("next").Add(float64(next)) |
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.
Have you considered putting these metrics inside TSDBBuilder.CompactAndUpload
(ie. make them a part of tsdbMetrics
collection). It feels this would require fewer changes in the overall code, and make "magic knowledge", like 2h block ranges, scoped closer together.
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.
That was my first try, but giving the information about current block end to TSDB did not look right. I am not too worried about the number of lines of change here because it's mostly replacing old stuff and only a minor addition of this function.
Signed-off-by: Ganesh Vernekar <[email protected]>
Signed-off-by: Ganesh Vernekar <[email protected]>
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.
🔥 Works for me
What this PR does
Adds a metric to determine how many blocks were produced for previous, current, and next block period w.r.t. the cycle end time.
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]
.about-versioning.md
updated with experimental features.