ingester: Stagger head compaction intervals when running ingesters in multiple zones #12090
Conversation
… multiple zones Signed-off-by: gotjosh <josue.abreu@gmail.com>
Signed-off-by: gotjosh <josue.abreu@gmail.com>
Signed-off-by: gotjosh <josue.abreu@gmail.com>
|
Can we do this without involving ring-consensus by just setting a different flag value on each zone's container? |
| @@ -3289,7 +3293,7 @@ func (i *Ingester) compactionServiceInterval() (firstInterval, standardInterval | |||
| // if ingest storage is enabled. | |||
| standardInterval = min(i.cfg.BlocksStorageConfig.TSDB.HeadCompactionIntervalWhileStarting, i.cfg.BlocksStorageConfig.TSDB.HeadCompactionInterval) | |||
| } else { | |||
| standardInterval = i.cfg.BlocksStorageConfig.TSDB.HeadCompactionInterval | |||
| standardInterval = i.calculateHeadCompactionInterval() | |||
| } | |||
|
|
|||
| if i.cfg.BlocksStorageConfig.TSDB.HeadCompactionIntervalJitterEnabled { | |||
There was a problem hiding this comment.
I think we have to rework the jitter logic. We want the jitter being applied after the per-zone time slicing. Let's say we have two zones and 20m interval. What we want is:
- zone-a: from time 0m to time 10m, with a jitter within that interval
- zone-b: from time 10m to time 20m, with a jitter within that interval
Currently we apply a 100% negative jitter, but that can also cause the overlapping between zones. I think we want a smaller jitter. The way I see it is:
- zone-a: compaction should run from time 0m to time 10m, but only start from time 0m to time 5m (that is a 50% positive jitter)
- zone-b: compaction should run from time 10m to time 20m, but only start from time 10m to time 15m (that is a 50% positive jitter)
| // calculateHeadCompactionInterval calculates the head compaction interval based on the number of zones | ||
| // and the configured head compaction interval to ensure we can stagger the compaction across zones. | ||
| // If zone awareness is not enabled, it returns the configured interval as-is. | ||
| func (i *Ingester) calculateHeadCompactionInterval() time.Duration { |
There was a problem hiding this comment.
I can't understand how this work without using time.Now() too. We want to synchronize the compaction between different ingesters. We need to take time.Now() in account too, to know how long we have to delay the 1st compaction.
| // After the first interval, we want the compaction to run at a specified interval for the partial zone if we have multiple zones, | ||
| // before we switch to running the compaction at the standard configured `HeadCompactionInterval`. | ||
| // If the criteria to have staggered compactions is not met, standardInterval and i.cfg.BlocksStorageConfig.TSDB.HeadCompactionInterval are the same. | ||
| stopTicker, tickerChan := util.NewVariableTicker(firstInterval, standardInterval, i.cfg.BlocksStorageConfig.TSDB.HeadCompactionInterval) |
There was a problem hiding this comment.
To my understanding standardInterval is now used to delay the recurring compactions (that is every i.cfg.BlocksStorageConfig.TSDB.HeadCompactionInterval). If so, I don't think this is the right approach, because when we apply the "delay" we don't also want to trigger a compaction once that delay has passed.
@colega We need to know how many total zones are and what are the actual values to make it deterministic. I think reading it from the ring is just easier. Don't see any real problem doing it. |
Can we do this without involving ring-consensus by just setting a different flag value on each zone's container? @colega We need to know how many total zones are and what are the actual values to make it deterministic. I think reading it from the ring is just easier. Don't see any real problem doing it. I don't think there's any problem in reading it from the ring, but IMO it's easier to reason when it's set statically on each zone. |
What this PR does
Attempts to calculate staggered intervals for ingesters in each zone to avoid ingesters across all zones to do head compaction at the same time, which in turn would make strong consistency queries fail.
Which issue(s) this PR fixes or relates to
Fixes #12081
TODO:
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]. If changelog entry is not needed, please add thechangelog-not-neededlabel to the PR.about-versioning.mdupdated with experimental features.Signed-off-by: gotjosh josue.abreu@gmail.com