Repository navigation
xds: Fail CDS cluster when EDS update has no usable endpoints - #13142
Open
lujiajing1126 wants to merge 1 commit into
Open
lujiajing1126 wants to merge 1 commit into
lujiajing1126 wants to merge 1 commit into
Conversation
When a previously READY EDS cluster received an update with no usable endpoints, CdsLoadBalancer2 passed the error to the delegate via handleNameResolutionError(). The priority/MultiChild subtree ignores that while READY, so the stale picker kept selecting endpoints the control plane had removed. Before ClusterResolverLB was merged into CdsLB2, this case published TRANSIENT_FAILURE and shut down the child policy. Use fail() for this case, and have fail() replace the shut-down GracefulSwitchLoadBalancer so a later update creates a fresh child policy instead of reusing the one that was shut down. Fixes grpc#13141 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the fix addressing stale READY pickers after usable endpoints are removed.
No open issues 🎉
What changed in this PR
Fixes #13141 by preventing xDS clusters from retaining a stale READY picker after an authoritative EDS update removes all usable endpoints.
Changes:
- Fail the cluster and shut down its child policy when no usable endpoints remain.
- Recreate the graceful-switch delegate after failure so later endpoint updates can recover.
- Add a regression test covering READY → empty endpoints → recovery with new addresses.
| File | Description |
|---|---|
| xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java | Tests failure, child shutdown, and recovery after endpoints return. |
| xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java | Uses fail() for empty endpoints and resets the delegate for recovery. |
Review effort: Balanced
This branch has not been deployed
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.
Fixes #13141
When a previously READY EDS cluster gets an update with no usable endpoints (e.g. an empty
ClusterLoadAssignment, or all endpoints unhealthy),CdsLoadBalancer2calleddelegate.handleNameResolutionError(). The priority/MultiChildLoadBalancersubtree ignores that while READY, so the stale picker kept selecting endpoints the control plane had removed. BeforeClusterResolverLoadBalancerwas merged intoCdsLoadBalancer2(ebb9420), this case publishedTRANSIENT_FAILUREand shut down the child policy.Changes:
fail()for the "No usable endpoint" case, so the cluster goes toTRANSIENT_FAILUREwith an error picker and the child policy is shut down.fail(), replace the shut-downGracefulSwitchLoadBalancerwith a new one, the same wayshutdown()does. Without this, a later non-empty update switches to the same priority factory, andGracefulSwitchLoadBalancerhands it to the child policy that was already shut down. This also applies to the existingfail()callers.Ambient xDS errors are stored separately by
XdsDependencyManagerand don't reach this branch, so only the authoritative "no usable endpoints" case changes. In-flight RPCs aren't affected, since subchannel shutdown is graceful.Added
ClusterResolverLoadBalancerTest.handleEdsResource_allEndpointsRemovedAfterReady_failsAndRecovers. It covers three steps: READY, then an empty EDS update, which should give TF plus the error picker and shut down the child; then endpoints coming back, which should create a new child with the new addresses.Note: I wasn't able to run the Gradle build locally (network restrictions in my environment), so I'm relying on CI for the test run. I did validate the
fail()change for the empty-endpoint branch end to end against 1.85.1 with a fake ADS server: stale picks went from 20/20 to 0/20, the RPCs failed fast withUNAVAILABLE, and the cluster recovered once endpoints came back.🤖 Generated with Claude Code