Skip to content

dns-discovery-netty: fix SRV resurrection during A-record backoff - #3596

Open
bryce-anderson wants to merge 1 commit into
apple:mainfrom
bryce-anderson:bl_anderson/dns-srv-removal-race
Open

dns-discovery-netty: fix SRV resurrection during A-record backoff#3596
bryce-anderson wants to merge 1 commit into
apple:mainfrom
bryce-anderson:bl_anderson/dns-srv-removal-race

Conversation

@bryce-anderson

Copy link
Copy Markdown
Contributor

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.

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.
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.

1 participant