fix(gorilla-merger): serve cold queries after restart (empty-head MinTime + non-blocking reload) - #342
Merged
Conversation
…Time + non-blocking reload) Cold parts were stored but a live cold query still returned empty after a restart, even with #341's MinTime floor in place. Two compounding causes: 1. customStore.timeRange() advertised MaxInt64 for an EMPTY tsdb head: tsdb.DB.StartTime() returns math.MaxInt64 when the head holds no samples (e.g. right after a restart, before the first warm fragment lands). That sentinel leaked into the StoreAPI's advertised MinTime, so thanos-query pruned the merger from EVERY query (no window can be >= MaxInt64) and streamColdSeries never ran. Take the MIN of the tsdb StartTime and the cold-part floor, and never advertise MaxInt64 (fall back to MinInt64 when the store is genuinely empty so it stays discoverable, returning no series). 2. ColdPartStore.Reload ran synchronously in main before the StoreAPI/HTTP servers started. It fetches + OpenParts every stored part, so a large accumulated cold tier (thousands of parts) blocked startup for tens of seconds — during which the gRPC endpoint was down and every query (warm and cold) returned empty. Run the reload in the background; the manifest is mutex-guarded, so concurrent queries see a growing manifest until it completes and the warm/open path serves immediately. Live: after a restart the StoreAPI now comes up ~80ms after WAL replay (vs ~53s before) and warm queries return data while the cold reload runs in the background; cold-only-window queries return the stored parts' series once reload completes (streamColdSeries emits N series). Adds a regression test asserting the empty-head MinTime never advertises MaxInt64 and drops to the cold floor when parts exist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cold parts were stored but a live cold query still returned empty after a merger restart, even with #341's MinTime floor in place and
streamColdSeriesdebug logs added. Investigation on the live multinode cluster (node1) found TWO compounding root causes, neither addressed by #341:customStore.timeRange()advertisedmath.MaxInt64for an EMPTY tsdb head.tsdb.DB.StartTime()returnsMaxInt64when the head holds no samples (verified empirically) — exactly the state right after a restart, before the first warm fragment lands, while cold parts already exist in S3. That sentinel leaked into the StoreAPI's advertisedMinTime, so thanos-query pruned the merger from every query (no window can be>= MaxInt64) andstreamColdSeriesnever ran — the served-empty symptom. Fix: advertise the MIN of the tsdb StartTime and the cold-part floor, and never advertiseMaxInt64(fall back toMinInt64when the store is genuinely empty so it stays discoverable and returns no series cheaply).ColdPartStore.Reloadblocked StoreAPI startup. It ran synchronously inmain()before the gRPC/HTTP servers started, fetching +OpenPart-ing every stored part. With a large accumulated cold tier (live: ~2000 parts), this took ~53s during which the merger served nothing (warm or cold). Fix: run the reload in the background; the manifest is mutex-guarded so concurrent queries see a growing manifest until it completes, and the warm/open path serves immediately.Live demonstration (node1)
series=1,streamColdSeries: emitting cold series cold_series=476After restart with the fixed image:
gorilla-merger uplogged ~80ms after WAL replay whilereloaded cold manifest(2014 parts) was still running in the background; warmcount(http_requests_total)returned data immediately; a cold-only deep-windowquery_rangereturned the stored parts' series and the merger loggedstreamColdSeries: emitting cold series cold_series=476.Test plan
go build ./...,go vet ./...,gofmt -lcleango test ./...greenTestCustomStoreTimeRangeEmptyHeadFAILS without thetimeRangefix (empty head advertised MinTime = MaxInt64; merger would be pruned) and PASSES with itstreamColdSeriesinvoked after a restart on the multinode cluster🤖 Generated with Claude Code