A75: refactor the aggregate cluster LB policy tree by mswierq · Pull Request #9423 · grpc/grpc-go

mswierq · 2026-09-18T11:42:31Z

Implements A75 for the aggregate cluster.

Follow-up to #9234. This change implements the new LB policy topology (priority -> outlier_detection -> cluster_impl -> priority -> xds_wrr_locality -> endpoint picking policy) for the aggregate cluster case by delegating leaf cluster configuration to buildLeafClusterConfig, and removes the legacy aggregate cluster configuration helpers.

RELEASE NOTES:

xds: Implement the aggregate cluster changes from gRFC A75. Each underlying
cluster of an aggregate cluster now uses the LB policy configured in that
cluster, and the LB policy configured in the aggregate cluster is ignored.
Outlier detection and other per-cluster settings now apply across all
priorities of the underlying cluster. The previous LB policy behavior can be
temporarily restored by setting the GRPC_XDS_AGGREGATE_CLUSTER_BACKWARD_COMPAT
environment variable to true; this mechanism will be removed in a future
release.

…d leaf priority policies

codecov · 2026-09-18T16:08:48Z

Codecov Report

❌ Patch coverage is 85.24590% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.70%. Comparing base (4599e77) to head (f668a43).

Files with missing lines	Patch %	Lines
internal/xds/balancer/cdsbalancer/configbuilder.go	81.25%	6 Missing ⚠️
internal/xds/balancer/cdsbalancer/cdsbalancer.go	89.65%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9423      +/-   ##
==========================================
- Coverage   87.79%   87.70%   -0.10%     
==========================================
  Files         437      437              
  Lines       31442    31387      -55     
==========================================
- Hits        27604    27527      -77     
- Misses       3838     3860      +22

Files with missing lines	Coverage Δ
internal/xds/balancer/cdsbalancer/cdsbalancer.go	`81.90% <89.65%> (+0.71%)`	⬆️
internal/xds/balancer/cdsbalancer/configbuilder.go	`92.75% <81.25%> (-2.93%)`	⬇️

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

easwars · 2026-09-18T22:14:54Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the CDS balancer configuration to align with gRFC A75, specifically updating the aggregate cluster configuration to wrap leaf clusters in outlier_detection and cluster_impl policies. It also cleans up test utilities and simplifies configuration building logic. A potential nil pointer dereference issue was identified in the EDS update logic, for which a defensive check has been suggested.

gemini-code-assist · 2026-09-18T22:17:29Z

 		priorities := [][]xdsresource.Locality{{}}
 		edsUpdate := leaf.clusterConfig.EndpointConfig.EDSUpdate
+		// Localities of length 0 is triggered by an NACK or resource-not-found
+		// error before update, or an empty localities list in an update. In either
+		// case want to create a priority, and send down empty address list, causing
+		// TF for that priority. "If any discovery mechanism instance experiences an
+		// error retrieving data, and it has not previously reported any results, it
+		// should report a result that is a single priority with no endpoints." -
+		// A37
 		if len(edsUpdate.Localities) != 0 {
 			priorities = groupLocalitiesByPriority(edsUpdate.Localities)
 		}


To prevent a potential nil pointer dereference, we should add a defensive check to ensure leaf.clusterConfig.EndpointConfig is not nil before accessing EDSUpdate. In xDS, the endpoint configuration might not be populated yet if the endpoints are not resolved or if there was an error resolving them.

Suggested change

priorities := [][]xdsresource.Locality{{}}

edsUpdate := leaf.clusterConfig.EndpointConfig.EDSUpdate

// Localities of length 0 is triggered by an NACK or resource-not-found

// error before update, or an empty localities list in an update. In either

// case want to create a priority, and send down empty address list, causing

// TF for that priority. "If any discovery mechanism instance experiences an

// error retrieving data, and it has not previously reported any results, it

// should report a result that is a single priority with no endpoints." -

// A37

if len(edsUpdate.Localities) != 0 {

priorities = groupLocalitiesByPriority(edsUpdate.Localities)

}

priorities := [][]xdsresource.Locality{{}}

var edsUpdate *xdsresource.EndpointsUpdate

if leaf.clusterConfig.EndpointConfig != nil {

edsUpdate = leaf.clusterConfig.EndpointConfig.EDSUpdate

}

// Localities of length 0 is triggered by an NACK or resource-not-found

// error before update, or an empty localities list in an update. In either

// case want to create a priority, and send down empty address list, causing

// TF for that priority. "If any discovery mechanism instance experiences an

// error retrieving data, and it has not previously reported any results, it

// should report a result that is a single priority with no endpoints." -

// A37

if edsUpdate != nil && len(edsUpdate.Localities) != 0 {

priorities = groupLocalitiesByPriority(edsUpdate.Localities)

}

I did some analysis and a nil EndpointConfig may actually happen. When one of the leaf clusters has not resolved its endpoints yet, but another one raises an error. However, instead of adding a simple nil check here, I fixed that a nil is not passed.

This is the commit that fixes it: 2e99573

@easwars @Pranjali-2501 please, let me know if it makes sense to deliver it with this PR or should I issue a new one?

Thanks for digging into this. It will be better to fix this bug and a test for this in a separate PR. It's a pre-existing bug and independent of the tree refactoring done as part of gRFC A75.

I prepared a new PR for this: #9518

…licy-tree-refactoring

Pranjali-2501 · 2026-10-11T12:10:46Z

-			continue
-		default:
-			return nil, nil, fmt.Errorf("unsupported cluster type %v for leaf cluster %q", clusterUpdate.ClusterType, clusterUpdate.ClusterName)
+		odCfg, endpoints, err := buildLeafClusterConfig(leaf, xdsLBPolicy)


https://screenshot-v2.corp.google.com/07p4t433u6dag
As per gRFC A75, for aggregate clusters we should use the LB policy configured in each underlying (leaf) cluster and ignore the one in the aggregate cluster.
A75 also asks for a temporary GRPC_XDS_AGGREGATE_CLUSTER_BACKWARD_COMPAT env var. When it's set to true, the leaves use the aggregate cluster's LB policy, i.e. the current behavior.

But we're still unmarshaling the LB policy of the root cluster and passing the same policy to every leaf here:

grpc-go/internal/xds/balancer/cdsbalancer/cdsbalancer.go

Lines 239 to 242 in 3d69670

// The LB policy is configured by the root cluster.

if err := json.Unmarshal(clusterConfig.Cluster.LBPolicy, &b.xdsLBPolicy); err != nil {

return b.annotateErrorWithNodeID(fmt.Errorf("error unmarshalling xDS LB Policy: %v", err))

}

So if the aggregate cluster uses the default round_robin and a leaf is configured with ring_hash, the leaf ends up using round_robin. None of the existing tests catch this because they all use the default policy for every cluster.

thank you for spotting that, I added the missing switch and using a leaf LB policy

I have also updated the RELEASE NOTES of this PR

Pranjali-2501 · 2026-10-12T09:48:55Z

I think we have decided to remove the outlierDetection config parsing from cdsbalancer.go and build it inside buildLeafClusterConfig instead of updating it for all priorities.

See this: #9234 (comment)

We have also decided to remove the outlierDetection field from leafClusterConfig.

this was fixed with a CDS on top of the leaf policy tree

Pranjali-2501 · 2026-10-12T15:31:07Z

-//	┌──────▼─────┐  ┌─────▼──────┐
-//	│xDSLBPolicy │  │xDSLBPolicy │ (Locality and Endpoint picking layer)
-//	└────────────┘  └────────────┘
+//	                 ┌────────┐


https://github.com/grpc/proposal/blob/master/A75_graphics/grpc_client_architecture_aggregate.png

According to the gRFC, for aggregate clusters, priority lb for each cluster should have a CDS child and than follows the same leaf cluster config.

Currently this PR creates outlier_detection as child of priority LB for each cluster.

Is this the decision taken, if so why?

The first implementation was doing an eager config building, which was rebuilding configs for all the leaves on each update, so CDS was not necessary.

I have reconsidered and decided to stick with A75 and C-Core implementation and I added CDS on the top of the leaf config tree.

Pranjali-2501 · 2026-10-12T15:58:23Z

Please resolve the merge conflicts in this PR.

…licy-tree-refactoring

This reverts commit 2e99573.

…olicy tree

mswierq · 2026-10-15T12:51:39Z

Please resolve the merge conflicts in this PR.

resolved

…licy-tree-refactoring

mswierq added 2 commits September 18, 2026 11:31

A75: refactor the aggregate cluster LB policy tree

e0f9227

Fix a deadlock on calling the same channel twice for the aggregate an…

2ade47f

…d leaf priority policies

easwars requested a review from Pranjali-2501 September 18, 2026 22:14

easwars assigned Pranjali-2501 Sep 18, 2026

easwars added the Type: Feature New features or improvements in behavior label Sep 18, 2026

easwars added this to the 1.85 Release milestone Sep 18, 2026

gemini-code-assist Bot reviewed Sep 18, 2026

View reviewed changes

eshitachandwani modified the milestones: 1.85 Release, 1.86 Release Sep 24, 2026

mswierq added 2 commits September 28, 2026 12:38

fix nil EndpointConfig

2e99573

Merge remote-tracking branch 'origin/master' into a75/aggregate-lb-po…

3d69670

…licy-tree-refactoring

Pranjali-2501 reviewed Oct 12, 2026

View reviewed changes

Pranjali-2501 assigned mswierq and unassigned Pranjali-2501 Oct 12, 2026

mswierq added 4 commits October 14, 2026 08:50

Merge remote-tracking branch 'origin/master' into a75/aggregate-lb-po…

a0d0137

…licy-tree-refactoring

Revert "fix nil EndpointConfig"

a008837

This reverts commit 2e99573.

Address feedback: fix using leaf/cluster LB policy switch

e604561

Address feedback: add a cds balancer at the top of the leaf cluster p…

e6075a5

…olicy tree

mswierq requested a review from Pranjali-2501 October 15, 2026 12:52

Merge remote-tracking branch 'origin/master' into a75/aggregate-lb-po…

a767cbc

…licy-tree-refactoring

Pranjali-2501 assigned Pranjali-2501 and unassigned mswierq Oct 17, 2026

Merge remote-tracking branch 'origin/master' into a75/aggregate-lb-po…

f668a43

…licy-tree-refactoring

	// The LB policy is configured by the root cluster.
	if err := json.Unmarshal(clusterConfig.Cluster.LBPolicy, &b.xdsLBPolicy); err != nil {
	return b.annotateErrorWithNodeID(fmt.Errorf("error unmarshalling xDS LB Policy: %v", err))
	}

Conversation

mswierq commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

easwars commented Sep 18, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pranjali-2501 commented Oct 12, 2026

Uh oh!

mswierq commented Oct 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mswierq commented Sep 18, 2026 •

edited

Loading

codecov Bot commented Sep 18, 2026 •

edited

Loading