dns-discovery-netty: fix SRV resurrection during A-record backoff - #3596
Open
bryce-anderson wants to merge 1 commit into
Open
dns-discovery-netty: fix SRV resurrection during A-record backoff#3596bryce-anderson wants to merge 1 commit into
bryce-anderson wants to merge 1 commit into
Conversation
Motivation: An SRV-rooted A-record resolution that fails with a transient error enters a backoff retry. That retry lived outside the publisher: the target was removed from aRecordMap for the duration of the backoff and a fresh ARecordPublisher (with empty state) was created per attempt. Discarding the per-attempt state caused two problems: - A concurrent SRV removal during the backoff window found no publisher to cancel, so the removal was lost and the pending retry resurrected a target DNS had reported as gone. - The last-known addresses could not be retracted when the target was removed mid-backoff, so the load balancer kept stale addresses until the whole SRV subscription was torn down. Modifications: - Fold the retry into AbstractDnsSubscription: on a resolution failure a backoff re-query is scheduled on the same subscription, preserving the last-known addresses across attempts. Gated by a retryResolveFailures flag, enabled only for SRV-rooted A lookups. NXDOMAIN still invalidates the addresses first; other failures keep them. - A single ARecordPublisher now serves a target for its whole lifetime and is registered in aRecordMap up front, so a concurrent SRV removal always finds it (even before the first subscribe or mid-backoff), cancels it, and retracts its last-known addresses. - Remove the external retryWhen wrapper, preserving its Reactive Streams Rule 2.13 wrapping of subscriber exceptions so those still revoke state. Result: A removed SRV target is no longer resurrected by an in-flight backoff retry, and its last-known addresses are retracted immediately on removal. A transient failure that recovers to the same addresses no longer emits a redundant AVAILABLE, matching a TTL re-query that returns an unchanged result.
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.
Motivation:
An SRV-rooted A-record resolution that fails with a transient error enters a backoff retry. That retry lived outside the publisher: the target was removed from aRecordMap for the duration of the backoff and a fresh ARecordPublisher (with empty state) was created per attempt. Discarding the per-attempt state caused two problems:
Modifications:
Result:
A removed SRV target is no longer resurrected by an in-flight backoff retry, and its last-known addresses are retracted immediately on removal. A transient failure that recovers to the same addresses no longer emits a redundant AVAILABLE, matching a TTL re-query that returns an unchanged result.