per tenant close_idle_tsdb_timeout by Shvejan · Pull Request #7813 · cortexproject/cortex

Shvejan · 2026-08-26T07:25:56Z

What this PR does:
continuation to this pr #7323, we are also making close_idle_tsdb_timeout to be configurable per tenant because queryIngestersWithin <= closeIdleTSDBTimeout

Which issue(s) this PR fixes:
Fixes #7160

Checklist

Tests updated
Documentation added
CHANGELOG.md updated - the order of entries should be [ENHANCEMENT]
docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags

yeya24

Thanks

SungJin1212 · 2026-09-01T06:16:04Z

@Shvejan
What if query_ingester_within=0s query_store_after=12h close_idle_tsdb_timeout=1h?

Current validation emits no error, but the T-11h ~ T range would be a hole.

Should we add a validation like:

closeIdleTSDBTimeout > 0 && queryStoreAfter > 0 && queryStoreAfter >= closeIdleTSDBTimeout

sandy2008

Docs and the JSON schema regenerate identically, and the targeted validation/ingester tests pass. A few things I noticed, mostly around upgrades; details inline.

sandy2008 · 2026-10-27T02:22:24Z

+	require.Equal(t, int64(1), i.TSDBState.seriesCount.Load())
+
+	// Wait a bit and confirm TSDB is NOT closed (timeout is disabled).
+	time.Sleep(500 * time.Millisecond)


This passes even without the new <= 0 guard in closeAndDeleteIdleUserTSDBs (I removed it locally): after 500ms the head hasn't been compacted yet (HeadCompactionIdleTimeout is 1s), so shouldCloseTSDB returns tsdbNotCompacted whatever the timeout. Since the close-idle loop now always runs, that guard is what keeps every tenant on the default 0 from having its TSDB closed once it's compacted and shipped, so it deserves a real test. Polling until db.Head().NumSeries() == 0 && db.getOldestUnshippedBlockTime() == 0, then waiting a few CloseIdleTSDBIntervals and asserting the TSDB is still there (and idleTsdbChecks{tsdbIdleClosed} is 0), makes it fail without the guard.

You're right, the 500ms sleep finished before head compaction, so it never exercised the guard. The test now waits until the head is compacted and every block is shipped, lets the close-idle loop run a few more intervals, and then checks that the TSDB still exists and that cortex_ingester_idle_tsdb_checks_total{result="idle_closed"} is 0. With the guard removed it fails, and so does PerTenantTimeout.

sandy2008 · 2026-10-27T02:22:24Z

+	// Per-tenant limit enables close-idle for this tenant
+	limits.CloseIdleTSDBTimeout = model.Duration(1 * time.Second)
+
+	tenantLimits := newMockTenantLimits(map[string]*validation.Limits{userID: &limits})


Nit: the defaults and the override for userID are the same value here, so this doesn't really exercise the per-tenant path. Default 0 plus an override only for userID (optionally with a second tenant whose TSDB must stay open) would.

Done. The default is now 0, and only userID has an override (1s). I added a second tenant on the default whose TSDB has to stay open after it's compacted and shipped.

sandy2008 · 2026-10-27T02:22:24Z

 			userID, shuffleShardingLookback, queryStoreAfter)
 	}

+	if queryIngestersWithin > 0 && shuffleShardingLookback > 0 && shuffleShardingLookback < queryIngestersWithin {


This check is new: before, the lookback only had to be ≥ query_store_after, and the "valid: shuffleShardingLookback between queryStoreAfter and queryIngestersWithin" case was flipped to invalid. It matches the flag help, so it's probably right, but since ValidateQueryLimits also runs in the runtime-config loader, an existing override with lookback < query_ingesters_within will make the whole overrides file fail to load after upgrading (fatal at startup; on reload the old config is kept). Could you call it out in the CHANGELOG, or split it into its own PR?

i took a second look and the validation check seems unnecessary removed it from this PR

sandy2008 · 2026-10-27T02:22:24Z

 						return nil, err
 					}
-					if err := ul.ValidateQueryLimits(userID, l.cfg.BlocksStorage.TSDB.CloseIdleTSDBTimeout); err != nil {
+					if err := ul.ValidateQueryLimits(userID, time.Duration(ul.CloseIdleTSDBTimeout)); err != nil {


Ingesters aren't in tenantLimitCheckTargets, so this check never runs there, but they're now the component that applies close_idle_tsdb_timeout. An override like close_idle_tsdb_timeout: 1h with query_ingesters_within: 24h and query_store_after: 12h is accepted by ingesters but rejected by queriers, so ingesters can close TSDBs whose data queriers still read only from ingesters. Should the ingester at least ignore (and log) a per-tenant value below query_ingesters_within?

i think this is an existing issue even with the older flag, i changed to fall back to 0 if the value of close_idle_tsdb_timeout is invalid for that tenant only and added a log for this case

sandy2008 · 2026-10-27T02:22:24Z

 * [FEATURE] Engine: Add `-querier.selector-batch-size` and `-ruler.selector-batch-size` flags to configure series batching in the Thanos promQL engine. 0 disables batching. #7763
 * [CHANGE] Ingester: Formally deprecate `-blocks-storage.tsdb.max-exemplars`, scheduled for removal in v1.24.0. Use the per-tenant `max_exemplars` limit instead. The flag still works as the global fallback when `max_exemplars` is 0, but setting it now logs a warning and increments `deprecated_flags_inuse_total`. #7793
-* [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). #7160
+* [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, `close_idle_tsdb_timeout` and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). #7323 #7813


-blocks-storage.tsdb.close-idle-tsdb-timeout / blocks_storage.tsdb.close_idle_tsdb_timeout are removed outright, and since the config is parsed strictly, deployments that set them will fail to start until they move to limits. #7323 did the same for the querier settings, but v1-guarantees.md asks to keep old flags and config working for two minor releases, so a deprecated alias may be worth it; at minimum the migration should be spelled out here, along with the new lookback validation and the fact that the close-idle loop now always runs (TSDBs of tenants marked for deletion get closed even with the limit at 0). Also, this edits the #7323 line, which now sits in ## 1.22.0 in progress on master, so after a rebase it likely needs its own [CHANGE] entry under ## master / unreleased.

sounds good, addressed

Signed-off-by: Shvejan Mutheboyina <[email protected]>

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

Shvejan requested a review from a team as a code owner August 26, 2026 07:25

Shvejan requested a review from SungJin1212 August 26, 2026 07:25

pull-request-size Bot added the size/L label Aug 26, 2026

dosubot Bot added component/ingester type/feature labels Aug 26, 2026

yeya24 approved these changes Aug 28, 2026

View reviewed changes

SungJin1212 approved these changes Sep 1, 2026

View reviewed changes

sandy2008 reviewed Oct 27, 2026

View reviewed changes

Shvejan added 2 commits November 11, 2026 06:41

per tenant close_idle_tsdb_timeout

6241eff

Signed-off-by: Shvejan Mutheboyina <[email protected]>

addressing comments

e59ddea

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

Shvejan force-pushed the per-tenant-query-configs branch from 2f75e22 to e59ddea Compare November 15, 2026 04:07

pull-request-size Bot added size/XL and removed size/L labels Nov 15, 2026

Conversation

Shvejan commented Aug 26, 2026

Uh oh!

yeya24 left a comment

Choose a reason for hiding this comment

Uh oh!

SungJin1212 commented Sep 1, 2026

Uh oh!

sandy2008 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants