Scheduled OOO Compaction without dependency on in-order head Compaction by MarkintoshZ · Pull Request #11847 · prometheus/prometheus

MarkintoshZ · 2023-01-13T02:35:49Z

This is my first time contributing so I am unsure if I did everything right. Pointers are much appreciated!

Is it necessary to add a test for the new behavior? If so, what is the best way to test the 2-hour scheduled OOO compaction behavior?

bboreham

Recommend you do include a test.

codesome

We need to make changes to not do OOO compaction every minute (more in below comments).

Additionally, right now we can predict when in-order and out-of-order compactions happen (i.e. in between the block durations, so every odd hours for a 2h block duration). It would be nice to preserve this predictability. We can trigger the OOO compaction in the middle of two aligned OOOCompactInterval. i.e. if OOOCompactInterval was 2h, then OOO compactions would be attempted at odd hours (while even hours are the "aligned" OOOCompactInterval), which should be around the time in-order compaction happens. We can achieve this by calculating the time remaining until next trigger after every OOO compaction trigger and use time.After.

codesome · 2023-02-02T07:20:28Z


+	// interval * 3 / 2 so that OOO compaction will happen around the same time as
+	// in-order compaction with default options
+	OOOScheduledCompact := time.NewTimer(db.opts.OutOfOrderCompactInterval * 3 / 2)


We need to actually align the time w.r.t. the wall clock. So in this case, the math would be like this:

OutOfOrderCompactInterval := 2 * time.Hour // The config. nowUnix := time.Now().Unix() oooCompactionIntvSec := int64(OutOfOrderCompactInterval / time.Second) // Aligned 'now'. In this example, an even hour. nextCompaction := (nowUnix / oooCompactionIntvSec) * oooCompactionIntvSec // Move the aligned 'now' to midway of the interval. In this example, the next odd hour. nextCompaction += oooCompactionIntvSec / 2 if nextCompaction < nowUnix { nextCompaction += oooCompactionIntvSec } timeUntilNextCompaction := time.Duration(nextCompaction-nowUnix) * time.Second // oooScheduledCompact := time.NewTimer(timeUntilNextCompaction) // Use this now. Note the OOO->ooo. fmt.Println("Now", time.Now().UTC()) fmt.Println("Waiting time", timeUntilNextCompaction) fmt.Println("Compaction will happen at", time.Now().Add(timeUntilNextCompaction).UTC())

The print statements and comments are for explaining. In the final code, you can say something like We align the compactions to happen with in-order compaction, which happens midway between aligned intervals of time.

Those make a lot of sense. When I implemented the change last time, I was actually quite confused about why we needed to align the compaction time if the alignment would change during the execution of the program (e.g. backoff from failing compaction). Now I understand what you mean by aligning with the compaction. Thank you for being extra patient with me!

MarkintoshZ · 2023-02-10T00:10:20Z

Since we do regular compaction and OOO compaction separately now, should we have two sets of compaction metric counters, one for regular and one for OOO?

bwplotka · 2023-05-24T17:00:38Z

We definitely need a counter with some label "type" that is "head" or "ooo" perhaps (: @MarkintoshZ

Good work on this, happy to help going forward with @jesusvazquez sorry for lag!

jesusvazquez · 2023-05-25T09:19:12Z

Since we do regular compaction and OOO compaction separately now, should we have two sets of compaction metric counters, one for regular and one for OOO?

Yes I think this a good call and also agree with Bartek's idea of using label values like head and ooo or head and ooohead.

Signed-off-by: Mark Zhang <[email protected]>

Signed-off-by: Mark <[email protected]>

Signed-off-by: Mark Zhang <[email protected]>

MarkintoshZ · 2023-06-19T17:05:57Z

I updated the db metrics to include both head and ooo completion counters. It's ready for review now @jesusvazquez :)

roidelapluie · 2023-12-05T11:57:07Z

cc @jesusvazquez would you please take a look at this pull request again?

Thanks!

ArthurSens

I've made some suggestions to how you're handling the new metrics.

Working with different labels, instead of different metrics, makes aggregation easier

ArthurSens · 2024-03-30T20:47:46Z

+	headCompactionsFailed    prometheus.Counter
+	headCompactionsTriggered prometheus.Counter
+	headCompactionsSkipped   prometheus.Counter
+	oooCompactionsFailed     prometheus.Counter
+	oooCompactionsTriggered  prometheus.Counter
+	oooCompactionsSkipped    prometheus.Counter


I wouldn't split the compaction metrics. Ideally, we keep the same metric but use different label values for each type of head.

Since we want new labels now, we would use prometheus.CounterVec instead of prometheus.Counter

Something like this:

- compactionsFailed prometheus.Counter - compactionsTriggered prometheus.Counter - compactionsSkipped prometheus.Counter + compactionsFailed prometheus.CounterVec + compactionsTriggered prometheus.CounterVec + compactionsSkipped prometheus.CounterVec

ArthurSens · 2024-03-30T21:01:24Z

-	m.compactionsTriggered = prometheus.NewCounter(prometheus.CounterOpts{
-		Name: "prometheus_tsdb_compactions_triggered_total",
-		Help: "Total number of triggered compactions for the partition.",


Here we need to keep the metric you just deleted, but we'll use NewCounterVec instead. For example:

m.compactionsTriggered = *prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "prometheus_tsdb_head_compactions_triggered_total", Help: "Total number of triggered compactions for the partition.", }, []string{"type"})

ArthurSens · 2024-03-30T21:03:53Z

+		case <-oooScheduledCompact.C:
+			oooScheduledCompact.Reset(db.opts.OutOfOrderCompactInterval)
+
+			db.metrics.oooCompactionsTriggered.Inc()


When increasing the metric, you'll want:

db.metrics.compactionsTriggered.WithLabelValues("ooohead").Inc()

or, when increasing the regular head metrics:

db.metrics.compactionsTriggered.WithLabelValues("head").Inc()

ArthurSens

Sorry for taking so long to review though, could we also ask you to resolve the conflicts? 😅

ArthurSens · 2024-03-30T21:08:20Z

+	// We align the compactions to happen with in-order compaction, which happens midway
+	// between aligned intervals of time.


Suggested change

// We align the compactions to happen with in-order compaction, which happens midway

// between aligned intervals of time.

// We align the out-of-order compactions to happen with in-order compaction, which happens midway

// between aligned intervals of time.

jesusvazquez

@MarkintoshZ this is close to completion, would you like to give it one last push?

jesusvazquez · 2024-10-17T08:42:59Z

@MarkintoshZ gentle reminder about this PR! We'll be cutting Prometheus 3.0 major release in a month and we'd love to have this in if you are still up for it!

bboreham · 2025-02-25T11:57:45Z

Hello from the bug-scrub! @MarkintoshZ do you think you will come back to this?

MarkintoshZ · 2025-02-26T20:31:40Z

Yes, I will try to fix this PR and get it up to date by this weekend

bboreham · 2025-08-26T11:28:00Z

Hello again from the bug-scrub! @MarkintoshZ we can hold this open since you indicated you would come back.
Please let us know if things have changed.

@poi1649 indicated they could pick it up.

bboreham · 2026-01-06T11:41:31Z

Hello again from the bug-scrub!

@poi1649, did you get anywhere? You commented at #11834 (comment)

MarkintoshZ requested a review from codesome as a code owner January 13, 2023 02:35

MarkintoshZ force-pushed the ooo-compaction branch from 7726448 to cf12151 Compare January 13, 2023 22:40

bboreham reviewed Jan 17, 2023

View reviewed changes

Comment thread tsdb/db.go Outdated

Comment thread tsdb/db.go Outdated

Comment thread tsdb/db.go Outdated

codesome requested changes Jan 23, 2023

View reviewed changes

Comment thread tsdb/db.go Outdated

Comment thread tsdb/db.go Outdated

MarkintoshZ requested a review from codesome January 29, 2023 23:13

codesome reviewed Feb 2, 2023

View reviewed changes

MarkintoshZ requested a review from codesome February 9, 2023 23:59

MarkintoshZ requested a review from jesusvazquez as a code owner May 25, 2023 17:42

MarkintoshZ force-pushed the ooo-compaction branch from dcfb6e9 to 28b5183 Compare May 25, 2023 17:43

MarkintoshZ force-pushed the ooo-compaction branch from 28b5183 to 10a4d46 Compare June 18, 2023 20:58

MarkintoshZ added 5 commits June 18, 2023 20:01

Scheduled OOO Compaction without dependency on in-order head Compaction

e19943d

Signed-off-by: Mark Zhang <[email protected]>

Fix OOO Scheduled Compaction & Add Test

0298f15

Signed-off-by: Mark <[email protected]>

Align OOO compaction with wall time

6e8ec3e

Signed-off-by: Mark <[email protected]>

Update compaction metrics for head and ooo

90b17e9

Signed-off-by: Mark Zhang <[email protected]>

Update unit tests for new db metrics

2e9b1f7

Signed-off-by: Mark Zhang <[email protected]>

MarkintoshZ force-pushed the ooo-compaction branch from 10a4d46 to 2e9b1f7 Compare June 19, 2023 02:07

accept linter recommandation using time.Until in tests

ed7288e

Signed-off-by: Mark Zhang <[email protected]>

MarkintoshZ force-pushed the ooo-compaction branch from 85bd0f3 to ed7288e Compare June 19, 2023 02:42

roidelapluie assigned jesusvazquez Jul 4, 2023

jesusvazquez mentioned this pull request Sep 6, 2023

Mark out-of-order as stable #12631

Closed

ArthurSens requested changes Mar 30, 2024

View reviewed changes

jesusvazquez reviewed Sep 6, 2024

View reviewed changes

ArthurSens mentioned this pull request Oct 31, 2024

Remove OOO Head compaction's dependency on the in-order Head compaction #11834

Open

anhtranbk mentioned this pull request Apr 3, 2026

tsdb: decouple OOO compaction from in-order head compaction #18434

Open

		// We align the compactions to happen with in-order compaction, which happens midway
		// between aligned intervals of time.

Conversation

MarkintoshZ commented Jan 13, 2023

Uh oh!

bboreham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codesome left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codesome Feb 2, 2023

Choose a reason for hiding this comment

Uh oh!

MarkintoshZ Feb 10, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MarkintoshZ commented Feb 10, 2023

Uh oh!

bwplotka commented May 24, 2023

Uh oh!

jesusvazquez commented May 25, 2023

Uh oh!

MarkintoshZ commented Jun 19, 2023

Uh oh!

roidelapluie commented Dec 5, 2023

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

ArthurSens Mar 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArthurSens Mar 30, 2024

Choose a reason for hiding this comment

Uh oh!

ArthurSens Mar 30, 2024

Choose a reason for hiding this comment

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

ArthurSens Mar 30, 2024

Choose a reason for hiding this comment

Uh oh!

jesusvazquez left a comment

Choose a reason for hiding this comment

Uh oh!

jesusvazquez commented Oct 17, 2024

Uh oh!

bboreham commented Feb 25, 2025

Uh oh!

MarkintoshZ commented Feb 26, 2025

Uh oh!

bboreham commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bboreham commented Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ArthurSens Mar 30, 2024 •

edited

Loading

bboreham commented Aug 26, 2025 •

edited

Loading