Skip to content

Fix/thanos query sidecar discovery - #1292

Merged
wmousa merged 3 commits into
R26.2-PREfrom
fix/thanos-query-sidecar-discovery
Sep 2, 2026
Merged

Fix/thanos query sidecar discovery#1292
wmousa merged 3 commits into
R26.2-PREfrom
fix/thanos-query-sidecar-discovery

Conversation

@wmousa

@wmousa wmousa commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

No description provided.

prometheus, thanos-sidecar and thanos-store are all `mode: global`, so a
3-manager swarm runs three of each. thanos-query was pointed at them by
bare service name:

    --store=thanos-store:10901
    --store=thanos-sidecar:10901

so it saw exactly one of each. Its own startup log shows it, printing one
line each instead of three:

    msg="adding new store with [storeEndpoints]"    address=thanos-store:10901
    msg="adding new sidecar with [storeEndpoints…]" address=thanos-sidecar:10901

Two separate prefixes are needed to fix it:

  * `dns+` makes Thanos treat the address as a DNS-discovered set and
    re-resolve it periodically, rather than as one static endpoint.
  * `tasks.` is required because these services use the default VIP
    endpoint mode, where the bare service name resolves to a single
    *virtual* IP. `dns+` alone would therefore still resolve to exactly one
    endpoint and fix nothing -- the routing mesh simply pins Thanos's
    long-lived gRPC connection to whichever task it first hit.
    `tasks.<service>` returns every task's real IP; prometheus.yml.j2
    already uses this idiom for tasks.node-exporter.

Why this caused "no data" alerts: the most recent ~2h of samples exist only
in each Prometheus's local TSDB and are served solely by *its own* sidecar;
only older blocks reach the object store. Grafana alert rules evaluate
recent data, so every evaluation depended on the single pinned sidecar.
When that sidecar was slow the query came back EMPTY rather than failing --
presenting as `noDataState: NoData` (empty result set) instead of
`execErrState: Error` -- and the alert self-cleared once it responded.

Seen on a customer cluster as recurring "no data" alerts whose underlying
Prometheus data was complete and healthy throughout
(scrape_samples_scraped a continuous 3.65k-5.15k, count(lvol_status_code)
a steady 90-140, never absent), alongside:

    "update of endpoint failed" err="getting metadata: fallback fetching
     info from thanos-sidecar:10901: rpc error: code = DeadlineExceeded"

Note this is only reproducible with 2+ manager nodes; on a single-manager
swarm there is one sidecar and the bug is invisible by construction.

Unrelated to the /cluster/metrics series leak fixed in #1281 -- a read-path
defect present since the monitoring stack was written. Same bug on main and
R26.3.
…edup

All three Prometheus replicas shared one hardcoded external label set:

    external_labels:
      monitor: 'codelab-monitor'

so every replica emitted series with byte-identical identity
({job, instance, monitor}). Two consequences:

  * thanos-query had nothing to deduplicate on, so a query merged three
    independent series claiming the same identity whose samples were taken
    at different seconds (observed: one replica scraping at :02, another at
    :56). The result is one interleaved, non-monotonic stream rather than a
    clean series -- unstable between evaluations for an alert rule using a
    `last` reducer over a short window.
  * all three sidecars uploaded blocks carrying identical external labels
    into the same bucket. Thanos treats those as overlapping and the
    compactor halts by default, consistent with a compactor that had done
    essentially nothing (18 MiB RSS, 0% CPU, 200 kB total network).

Thanos identifies HA replicas by a label that differs per replica and drops
it when deduplicating, so:

  * prometheus.yml.j2 gains `replica: '${REPLICA}'`;
  * prometheus gains --enable-feature=expand-external-labels, which expands
    ${REPLICA} from the environment. The indirection is required because
    prometheus.yml is bind-mounted -- every replica reads identical bytes and
    cannot self-identify from file content;
  * the compose sets REPLICA per task from the swarm node-hostname template;
  * thanos-query gains --query.replica-label=replica;
  * thanos-compactor gains --deduplication.replica-label=replica so it
    vertically compacts the overlap instead of halting on it.

Note for anyone editing prometheus.yml.j2: it is Jinja-rendered before
deployment, so a swarm template written literally with double braces in that
file is evaluated as a Jinja expression and breaks rendering. The swarm
template therefore lives in the compose file and the placeholder here is the
shell-style ${REPLICA}.

Verified: the template renders under Jinja with ${REPLICA} intact and
basic_auth substitution unaffected, and the compose parses with the expected
flags on all four services.
thanos-compactor was `mode: global`, so a 3-manager swarm ran three
compactors against the same object-storage bucket. Thanos requires exactly
one compactor per bucket; concurrent compactors rewriting the same blocks is
a documented data-corruption risk, and with halt-on-overlap the three also
contend for the same work.

More pressing after the previous commit: with
--deduplication.replica-label set the compactor performs vertical compaction
of the replicas' overlapping blocks, so three racing on the same blocks is
precisely the case Thanos warns about.

thanos-store and thanos-query stay `mode: global` -- both read-only and safe
to scale; only the compactor must be a singleton.
@wmousa
wmousa force-pushed the fix/thanos-query-sidecar-discovery branch from 03b17c1 to d7f3943 Compare September 1, 2026 21:45

@Hamdy-khader Hamdy-khader left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@wmousa
wmousa merged commit f354680 into R26.2-PRE Sep 2, 2026
5 of 8 checks passed
@wmousa
wmousa deleted the fix/thanos-query-sidecar-discovery branch September 2, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants