compaction: Separate metrics for tracking retention and compaction #11263
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Trivy scan found the following vulnerabilities:
|
ashwanthgoli
approved these changes
Nov 20, 2023
pkg/compactor/compactor.go
Outdated
| @@ -614,6 +616,8 @@ func (c *Compactor) CompactTable(ctx context.Context, tableName string, applyRet | |||
| // wait for lock to be released since we can't mark delete requests as processed without checking all the tables | |||
| select { | |||
| case <-lockWaiterChan: | |||
| // refresh index list cache since the compaction would have changed the index files in the object store | |||
| sc.indexStorageClient.RefreshIndexTableNamesCache(ctx) | |||
There was a problem hiding this comment.
we refresh the table here, is this still required when compacting a table?
There was a problem hiding this comment.
ahh, right, this one is not required. Will remove it.
pkg/compactor/compactor.go
Outdated
| @@ -126,6 +126,8 @@ func (cfg *Config) Validate() error { | |||
|
|
|||
| if cfg.ApplyRetentionInterval == 0 { | |||
| cfg.ApplyRetentionInterval = cfg.CompactionInterval | |||
| // add some jitter to avoid running retention and compaction at same time | |||
| cfg.ApplyRetentionInterval += cfg.CompactionInterval / 2 | |||
There was a problem hiding this comment.
nit: should we instead add a jitter to when the first retention kicks in?
There was a problem hiding this comment.
I would like to add jitter only if both retention and compaction have the same frequency. I will update the code a bit.
jeschkies
pushed a commit
that referenced
this pull request
Nov 21, 2023
…11263) **What this PR does / why we need it**: In PR #9884, we separated the retention loop from compaction to avoid blocking compaction for too long due to some intensive delete requests. Currently, we track retention and compaction using the same metrics. This PR adds separate metrics for monitoring retention operation. I have also updated the Retention dashboard to use the new metrics.
JoaoBraveCoding
added a commit
to JoaoBraveCoding/loki
that referenced
this pull request
Nov 28, 2023
With this update we get: - Reworked retention dashboards done in grafana#11263 as a follow-up to grafana#9884 - New write dashboards for structured metadata done in grafana#11087
8 tasks
rhnasc
pushed a commit
to inloco/loki
that referenced
this pull request
Apr 12, 2024
…rafana#11263) **What this PR does / why we need it**: In PR grafana#9884, we separated the retention loop from compaction to avoid blocking compaction for too long due to some intensive delete requests. Currently, we track retention and compaction using the same metrics. This PR adds separate metrics for monitoring retention operation. I have also updated the Retention dashboard to use the new metrics.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
In PR #9884, we separated the retention loop from compaction to avoid blocking compaction for too long due to some intensive delete requests. Currently, we track retention and compaction using the same metrics. This PR adds separate metrics for monitoring retention operation. I have also updated the Retention dashboard to use the new metrics.