Feat/tsdb/close stale tsdb

[thorfour]

What this PR does: Closes and removes TSDB's that haven't been written to in a given time period.

Which issue(s) this PR fixes: This cleans up disk and memory footprint for open TSDB's that no longer are written to.

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[pracucci]

I see the use case for this feature (which I agree on), but I think the current implementation may lead to data loss (see comment). I've also left few minor comments to help improving the readability of code.

[pracucci]

Good job @thorfour ! I believe this feature is a nice to have but, at the same time, the implementation is still a bit complex (intrinsically complex).

As mentioned offline, an option to simplify it may be changing a bit the meaning of the config option. What if we allow to set for how long the TSDB head should be empty before the TSDB is released? It would not allow to release a TSDB before the block range period + threshold, BUT will remove the snapshotting at all because if the head is empty there's nothing to snapshotting and the code would simplify.

[codesome]

Related to this: prometheus/prometheus#6637

[pracucci]

I've just merged the PR #1982. Please be aware we should now decrease the memUsers metric whenever a TSDB is removed.

[codesome]

@thorfour: I just discussed with @pracucci offline about TSDB not compacting the last remaining data from Head after ingestion stops and possible solution in cortex (See prometheus/prometheus#6637 as to why it won't be inbuilt in TSDB).

Here is what we have come up with as a solution (for TSDB which did not receive any sample for past X duration):

1. Reject appends on TSDB (blocked on the Cortex side)
2. Snapshot Head (I will add this to TSDB if this flow makes sense)
3. Move the new block to original data dir
4. Truncate Head (to remove data from memory) and reload the blocks.
5. Wait until the shipper has shipped all blocks to the storage (including the new one)
6. Close TSDB
7. Delete data from disk

WDYT?

[pracucci]

@thorfour @codesome Another option, probably easier but less efficient, would be done like Thanos (see code here): close TSDB, open it in read only, call FlushWAL(). Given having stale TSDBs shouldn't be the common case, we may trade efficiency with simplicity and follow Thanos' approach?

[codesome]

In a case where user has stopped sending data but is still querying, it would lead to some downtime (EDIT: not downtime, but gaps) for the user depending on how long the WAL replay+flush takes. (Maybe we can take care of that if we actually see that happening often in real-world)

[pracucci]

In a case where user has stopped sending data but is still querying, it would lead to some downtime (EDIT: not downtime, but gaps) for the user depending on how long the WAL replay+flush takes. (Maybe we can take care of that if we actually see that happening often in real-world)

True, but re-thinking it maybe it's an edge case we could accept as far as we document it. Moreover all this logic should be able to disable setting the option to 0 (and maybe it could be a good thing having it disabled by default?).

[thorfour]

@thorfour: I just discussed with @pracucci offline about TSDB not compacting the last remaining data from Head after ingestion stops and possible solution in cortex (See prometheus/prometheus#6637 as to why it won't be inbuilt in TSDB).

Here is what we have come up with as a solution (for TSDB which did not receive any sample for past X duration):

1. Reject appends on TSDB (blocked on the Cortex side)

2. Snapshot Head (I will add this to TSDB if this flow makes sense)

3. Move the new block to original data dir

4. Truncate Head (to remove data from memory) and reload the blocks.

5. Wait until the shipper has shipped all blocks to the storage (including the new one)

6. Close TSDB

7. Delete data from disk

WDYT?

This is similar to what I originally had implemented. I believe it works, but it's unfortunately complex code especially in the failure cases. We also have to go back to using a timestamp on writes to determine how long since a TSDB was last written which is unfortunate.

[thorfour]

@codesome what if we added an optional parameter to the Close function in TSDB or maybe a new function like CloseWithCompaction that cuts a new block from the head while closing?

[codesome]

@thorfour Maybe we can do the following which would not require this hack in place for upstream Prometheus: Refactor the db.Compact() into CompactHead() and CompactBlocks() (while Compact() calling these 2 methods). With that you can call CompactHead() in cortex and close the TSDB. Would that work for you?

[pracucci]

Maybe we can do the following which would not require this hack in place for upstream Prometheus: Refactor the db.Compact() into CompactHead() and CompactBlocks() (while Compact() calling these 2 methods). With that you can call CompactHead() in cortex and close the TSDB. Would that work for you?

@codesome As far as CompactHead() will check db.head.compactable() it will not work. The Compact() function is fine for us, but we would need a way to bypass db.head.compactable(): is there anything else we may need, that comes to your mind, to compact the entire head to a block?

[codesome]

As far as CompactHead() will check db.head.compactable() it will not work.

Yes I am aware of that. The plan is to have that check in Compact() while CompactHead() would take a rangeHead (which I will expose) and compact it without any checks.

[pracucci]

As far as CompactHead() will check db.head.compactable() it will not work.

Yes I am aware of that. The plan is to have that check in Compact() while CompactHead() would take a rangeHead (which I will expose) and compact it without any checks.

Right. Then I think your proposed solution may work.

[thorfour]

@codesome as long is it bypasses the head compatible check I think that solution would work!

[codesome]

PR for breaking the Compact() method into parts prometheus/prometheus#6820

[stale]

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

[thorfour]

While this feature is still needed, we need Thanos to vendor the latest Prometheus changes mentioned above, and then we need to vendor Thanos. Closing until these things are ready.