Skip to content

query: retry initial DNS resolution instead of becoming ready with missing endpoints - #9033

Open
shyundev wants to merge 1 commit into
thanos-io:mainfrom
shyundev:query-initial-dns-retry
Open

shyundev wants to merge 1 commit into
thanos-io:mainfrom
shyundev:query-initial-dns-retry

Conversation

@shyundev

@shyundev shyundev commented Sep 17, 2026

Copy link
Copy Markdown
  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Initial non-group DNS resolution is attempted once, and a failure is only logged, allowing the querier to become ready with a partial or empty endpoint list.

Move this resolution into the run group, retrying every 2s until an attempt returns no errors or shutdown cancels the wait. Remove the readiness timeout fallback (30s by default): the querier becomes ready after successful resolution and completion of the first endpoint update. Info requests in that update use --endpoint.info-timeout.

NXDOMAIN and empty answers count as resolved. Rule does not gate readiness on the endpoint set, so its first endpoint update starts no later than --query.sd-dns-interval, using the addresses resolved by then.

This follows the retry suggestion in #8334, using DNS errors because Update() returns none.

Fixes #9032

Verification

TestSetupEndpointSetInitialDNS uses testing/synctest to cover prolonged DNS failures, recovery, empty answers, NXDOMAIN, Rule's bounded wait and cancellation during an active lookup.

Using the #9032 Docker setup with the nameserver started 75s after the querier, a querier from this branch stays 503 on /-/ready until DNS recovers and then turns 200 with the endpoint listed without a restart; one from the PR base becomes ready with no endpoint. SIGTERM during the retries exits cleanly.

@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.62%. Comparing base (e3bceed) to head (043c9ca).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
cmd/thanos/query.go 83.33% 1 Missing ⚠️
cmd/thanos/rule.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9033      +/-   ##
==========================================
+ Coverage   64.56%   64.62%   +0.06%     
==========================================
  Files         289      289              
  Lines       37410    37423      +13     
==========================================
+ Hits        24155    24186      +31     
+ Misses      11157    11148       -9     
+ Partials     2098     2089       -9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shyundev

Copy link
Copy Markdown
Author

@GiedriusS could you take a look when you have a moment? A transient DNS failure at startup leaves the querier ready with missing endpoints, answering 200 with no warning. Repro in #9032.

@GiedriusS

GiedriusS commented Sep 18, 2026

Copy link
Copy Markdown
Member

So with this change it will retry a few times and then give up again if errors continue. At what point we can be certain that there will be no missing endpoints?

@shyundev
shyundev force-pushed the query-initial-dns-retry branch from b97debf to 7606b27 Compare September 20, 2026 14:12
@pull-request-size pull-request-size Bot added size/L and removed size/S labels Sep 20, 2026
…ssing endpoints

Initial non-group DNS failures were only logged, allowing Query to
become ready with a partial or empty endpoint list.

Retry the resolution every 2s until an attempt returns no errors or
shutdown cancels the wait, and remove the readiness timeout fallback so
Query becomes ready only after resolution succeeds and the first
endpoint update completes.

Rule does not gate readiness on the endpoint set, so its first update
starts no later than --query.sd-dns-interval.

Signed-off-by: Sang Hyun Lee <shyundev@gmail.com>
@shyundev
shyundev force-pushed the query-initial-dns-retry branch from 7606b27 to 043c9ca Compare September 20, 2026 14:27
@shyundev

shyundev commented Sep 20, 2026

Copy link
Copy Markdown
Author

You're right, the previous version could still give up. I've pushed a revision that retries initial resolution every 2s until success or shutdown, keeping the querier unready.

Before marking the querier ready, we now require the initial DNS resolution to return no errors and the first endpoint update to complete. NXDOMAIN and empty answers still do not count as errors.

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.

query: a failed initial DNS resolution still lets the querier become ready with a partial endpoint list

2 participants