Skip to content

Querier: isolate bucket-scan finder meta cache from a co-located store-gateway - #7748

Draft
sandy2008 wants to merge 2 commits into
cortexproject:masterfrom
sandy2008:fix/7727-querier-meta-cache-isolation
Draft

Querier: isolate bucket-scan finder meta cache from a co-located store-gateway#7748
sandy2008 wants to merge 2 commits into
cortexproject:masterfrom
sandy2008:fix/7727-querier-meta-cache-isolation

Conversation

@sandy2008

Copy link
Copy Markdown
Contributor

What this PR does:

When the bucket index is disabled and the querier and store-gateway run in the same process sharing -blocks-storage.bucket-store.sync-dir, both components cache block metas under the identical per-tenant path <sync-dir>/<tenant>/meta-syncer/, and each deletes that shared state based on its own view of which tenants are live:

  • Store-gateway → querier (repeats every sync interval): with store-gateway sharding enabled, deleteLocalFilesForExcludedTenants removes <sync-dir>/<tenant> for every tenant outside the local shard. The co-located querier's bucket-scan finder is unsharded, so its on-disk meta cache is deleted again on every sync — permanently defeating it. (Since the in-memory fetcher cache is consulted first, the practical cost lands on the querier's next restart, which becomes a full cold start: one meta.json GET per block.)
  • Querier → store-gateway (since fix(querier): evict per-tenant metadata fetchers for inactive tenants #7573, one-shot per deleted tenant): the finder evicts per-tenant state against the active set only and removes <sync-dir>/<tenant>/meta-syncer — while the co-located store-gateway still serves active and deleting tenants from that very directory. The comment claiming "we never reach into a co-located store-gateway's cache" was wrong in single-binary mode. (The deleting window is typically minutes with a healthy blocks cleaner, but unbounded if a tenant's deletion is stuck.)

The fix gives the querier's finder its own reserved root inside the sync dir: metas now live under <sync-dir>/__querier__/<tenant>/meta-syncer/. Neither component can reach into the other's directories anymore, which fixes both directions structurally:

  • pkg/util/users: QuerierMetaCacheDirName = "__querier__" is defined next to GlobalMarkersDir and reserved the same way __markers__ is — rejected by tenant-ID validation (CheckTenantIDIsSupported) and skipped by the users scanner — so nothing can ever treat it as a tenant. This is an explicit reservation rather than a lexical trick: bucket prefixes are never run through tenant-ID validation, and tenant IDs created before the character allowlist (Validate tenantID on single resolver #6727) could contain characters the allowlist now forbids, so only the skip-list + validation combination actually closes the collision surface.
  • pkg/querier: the finder creates fetchers and evicts caches under the reserved root, and sweeps its own root against the active set on every scan (the same shape the compactor uses for its meta-sync dirs) — this also reclaims directories left behind by a previous process; the sweep is a single ReadDir, negligible next to the bucket scan that precedes it. The wrong comment is replaced with the isolation rationale.
  • pkg/storegateway: deleteLocalFilesForExcludedTenants skips the reserved directory. This skip is required, not optional: any directory nested under the sync dir would otherwise look like a tenant dir and be wiped every sync (this is also why the issue's <sync-dir>/querier/ suggestion doesn't work on its own).

Design decisions, stated for review:

  • Why not just reconcile the querier's eviction against active+deleting (the issue's other suggestion)? That fixes only the milder, one-shot direction and leaves the repeating one. It would also retain fetchers, registries and disk caches for tenants the finder never scans (it scans active tenants only), partially un-doing the bounded-resource cleanup fix(querier): evict per-tenant metadata fetchers for inactive tenants #7573 added — and once the roots are separate, active-only reconciliation of the querier's own cache is simply correct. It also stays consistent with the metas-map pruning in Querier: prune bucket-scan finder per-tenant metadata for departed tenants on partial scan errors #7747, which reconciles against the same active set.
  • Why nested-reserved-name rather than a sibling directory next to sync-dir? The sync dir is commonly a dedicated volume mount point; a sibling directory would land outside the mount on the container's writable layer.
  • Reserved-name policy: a hypothetical pre-existing tenant literally named __querier__ (creatable only before tenant-ID validation was tightened) is now skipped by the scanner rather than scanned — the same policy the __markers__ reservation already applies. An out-of-band bucket prefix with this name is likewise ignored rather than treated as a tenant.
  • Upgrade/rollback: on upgrade the querier lazily re-populates its cache under the new root (one-time cold meta fetch, identical to a querier starting with an empty sync dir). Old <sync-dir>/<tenant>/meta-syncer dirs are deliberately not migrated or cleaned: in single-binary mode they are the store-gateway's live cache, and a querier-only process cannot distinguish its own legacy cache from a co-located store-gateway's. In sharded store-gateway deployments the old dirs are reclaimed by the existing exclusion cleanup; in querier-only deployments a small meta.json-only residue remains. On rollback, the old binary resumes using the old paths and __querier__/ is orphaned (and reclaimed by a sharded store-gateway's cleanup, which in the old binary doesn't skip it).
  • Only the non-default bucket-index-disabled read path is affected. No flags or config options change.

Testing: red→green regression tests cover (1) a deleting tenant's store-gateway meta-syncer cache surviving the querier's eviction (fails on master), (2) the querier's cache under __querier__/ surviving the store-gateway's exclusion cleanup, exercised against a real sharded BucketStores (fails on master), and (3) the finder's own-root sweep reclaiming stale tenant dirs left by a previous process. The reserved name's rejection as a tenant ID is pinned in pkg/util/users alongside the __markers__ case, and the pre-existing "don't delete sibling block data" guard is retained. Full pkg/querier, pkg/util/users, pkg/storage/tsdb and pkg/storegateway suites pass with -tags "netgo slicelabels" and with -race. An upgrade simulation (old-layout dirs + new binary) was exercised during review: old store-gateway state survives, the new root is reclaimed on inactivity.

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

Checklist

  • Tests updated
  • Documentation added (n/a — no config or flag changes; the reserved directory is documented on the constant)
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX] (labelled [CHANGE] per precedent for on-disk layout moves, with the upgrade note)
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags (n/a)

Per the Generative AI Contribution Policy: this change was developed with substantial AI assistance (multiple independent AI implementations reconciled and then adversarially cross-reviewed, under my direction). I have reviewed and validated all of it and take responsibility for its correctness.

…e-gateway

With the bucket index disabled and the querier and store-gateway running
in the same process sharing -blocks-storage.bucket-store.sync-dir, both
components cached block metas under the identical per-tenant path
<sync-dir>/<tenant>/meta-syncer/ and deleted that shared state based on
their own, different views of which tenants are live:

- With store-gateway sharding enabled, deleteLocalFilesForExcludedTenants
  removes <sync-dir>/<tenant> for every tenant outside the local shard on
  every sync, permanently defeating the co-located unsharded querier
  finder's disk cache (every querier restart becomes a cold start).
- Since cortexproject#7573 the finder evicts against the active tenant set only and
  removes <sync-dir>/<tenant>/meta-syncer, while the co-located
  store-gateway still serves active plus deleting tenants from that very
  directory. The comment claiming the finder never reaches into a
  co-located store-gateway's cache was wrong in single-binary mode.

The finder now keeps its metas under its own reserved root,
<sync-dir>/__querier__/<tenant>/meta-syncer/, so neither component can
reach into the other's directories, fixing both directions:

- users.QuerierMetaCacheDirName ("__querier__") is defined next to
  GlobalMarkersDir and reserved the same way __markers__ is: rejected by
  tenant ID validation and skipped by the users scanner, so nothing can
  treat it as a tenant. The explicit reservation matters because bucket
  prefixes are never run through tenant ID validation, and tenant IDs
  created before the character allowlist could contain characters the
  allowlist now forbids.
- The finder creates fetchers and evicts caches under the reserved root
  and sweeps its own root against the active set on every scan (the same
  shape the compactor uses), which also reclaims directories left behind
  by a previous process for the cost of one ReadDir.
- The store-gateway's exclusion cleanup skips the reserved directory.

On upgrade the querier lazily re-populates its cache under the new root
(a one-time cold meta fetch). Old <sync-dir>/<tenant>/meta-syncer
directories are deliberately not migrated or cleaned up: in single-binary
mode they are the store-gateway's live cache, and a querier cannot
distinguish its own legacy cache from a co-located store-gateway's.

Fixes cortexproject#7727

Signed-off-by: Sandy Chen <Yuxuan.Chen@morganstanley.com>
Signed-off-by: Sandy Chen <Yuxuan.Chen@morganstanley.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Querier bucket-scan blocks finder and store-gateway wipe each other's meta-syncer cache in single-binary mode

1 participant