xds/resolver: ensure OnCommitted is invoked on early stream creation failure to prune unreferenced clusters. by Pranjali-2501 · Pull Request #9140 · grpc/grpc-go

Pranjali-2501 · 2026-05-25T10:05:36Z

This PR handles the cluster ref count leaks in case of stream creation failure.

When an xDS ConfigSelector selects a cluster or cluster specifier plugin for an RPC, it increments the cluster's reference count to keep the cluster active in the Service Config while the RPC is in flight. Previously, if stream creation failed early (e.g., aborted inside an xDS HTTP filter or interceptor before the stream was committed), or in OnCommitted would never be called, leaving the cluster reference count permanently elevated and preventing old/removed clusters from being pruned from the Service Config.

This PR ensures OnCommitted is reliably invoked even when stream creation fails early, and makes the callback idempotent so cluster reference counts are decremented exactly once per RPC.

Key Changes

stream.go (newClientStream):

Automatically append an OnFinish CallOption wrapping rpcConfig.OnCommitted whenever OnCommitted is configured by the ConfigSelector.
Ensure opts are properly passed through newStream when interceptors are present so OnFinish runs if stream creation aborts early.

internal/xds/resolver/serviceconfig.go (configSelector.SelectConfig):

Wrap config.OnCommitted with sync.Once so that reference count decrements are idempotent across both normal stream commitment (onCommit) and stream termination (OnFinish calloption).
Reliably trigger unsubscribe() / sendNewServiceConfig() when a cluster or plugin's reference count drops to zero.

RELEASE NOTES:

xds/resolver: fix cluster reference count leaks by ensuring cluster counts are correctly decremented via OnFinish even if stream creation fails early.

codecov · 2026-05-25T10:26:06Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.09%. Comparing base (2a112a8) to head (654109c).
⚠️ Report is 25 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9140      +/-   ##
==========================================
- Coverage   83.33%   83.09%   -0.25%     
==========================================
  Files         420      422       +2     
  Lines       34028    34862     +834     
==========================================
+ Hits        28358    28969     +611     
- Misses       4253     4395     +142     
- Partials     1417     1498      +81

Files with missing lines	Coverage Δ
internal/xds/resolver/serviceconfig.go	`91.73% <100.00%> (ø)`
stream.go	`82.46% <100.00%> (+0.50%)`	⬆️

... and 48 files with indirect coverage changes

🚀 New features to boost your workflow:

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

eshitachandwani · 2026-07-10T09:38:24Z

+}
+
+func (icc *interceptingClientConn) UpdateState(state resolver.State) error {
+	return icc.ClientConn.UpdateState(state)


Nit : we can remove this function since clientconn is already embedded and this is just passthrough function.

eshitachandwani · 2026-07-10T09:45:40Z

+	// been prematurely removed.
+	select {
+	case js := <-jsonCh:
+		compareJSONConfigs(t, js, wantServiceConfig(clusterA, clusterB))


Nit: I think we should return error from here , since config update is not expected.

eshitachandwani · 2026-07-10T09:51:40Z

+	}
+
+	// Create an intercepting resolver builder.
+	jsonCh := make(chan string, 10)


Nit: Do we need a channel of size 10 ?

eshitachandwani

LGTM with some nits.

mbissa

LGTM. Validated the approach and test scenarios solve the usecases we need.

Pranjali-2501 · 2026-07-10T10:01:45Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the management of cluster and plugin reference counts during RPC execution, ensuring that the OnCommitted callback is idempotent and executed on stream termination even if stream creation fails early. It also introduces comprehensive end-to-end tests for delayed cluster removal. Feedback on the changes points out a potential source of test flakiness in the new end-to-end test due to non-deterministic channel selection when unblocking concurrent RPCs, and suggests a robust way to handle either RPC finishing first.

easwars · 2026-07-14T23:18:11Z

+	}
+
+	// Create an intercepting resolver builder.
+	jsonCh := make(chan string, 4)


Why do we need a buffer of 4 here?

We can go with a buffer of 3 here.

We only need a buffer of 1. The test is reading items out of the channel before the next item is enqueued. So, I don't understand why it needs to have a buffer of anything greater than 1.

This applies to other places where this channel is defined as well.

easwars · 2026-07-27T18:16:46Z

+	}
+
+	// Create an intercepting resolver builder.
+	jsonCh := make(chan string, 4)


We only need a buffer of 1. The test is reading items out of the channel before the next item is enqueued. So, I don't understand why it needs to have a buffer of anything greater than 1.

easwars · 2026-07-27T18:17:15Z

+	}
+
+	// Create an intercepting resolver builder.
+	jsonCh := make(chan string, 4)


This applies to other places where this channel is defined as well.

easwars · 2026-07-27T18:33:06Z

Please handle the minor nit in the test and please add a release note before merging.

newstream changes

26fd795

Pranjali-2501 added this to the 1.82 Release milestone May 25, 2026

Pranjali-2501 added Type: Feature New features or improvements in behavior Area: xDS Includes everything xDS related, including LB policies used with xDS. labels May 25, 2026

Pranjali-2501 added 2 commits May 25, 2026 03:15

Merge branch 'master' into newstream-changes

fad69ad

update test

cad02d6

Pranjali-2501 requested a review from easwars May 25, 2026 10:41

Pranjali-2501 assigned easwars May 25, 2026

easwars reviewed May 26, 2026

View reviewed changes

Comment thread internal/resolver/config_selector.go Outdated

easwars assigned Pranjali-2501 and unassigned easwars May 26, 2026

Pranjali-2501 requested a review from easwars May 27, 2026 14:29

Pranjali-2501 assigned easwars and unassigned Pranjali-2501 May 27, 2026

Pranjali-2501 mentioned this pull request May 27, 2026

xds/gcp_authn: implement GCP Authentication filter (gRFC A83). #9119

Merged

easwars assigned Pranjali-2501 and unassigned easwars May 27, 2026

Pranjali-2501 assigned easwars and unassigned Pranjali-2501 May 28, 2026

remove onCommitted from RPCConfig

2fac787

Pranjali-2501 changed the title ~~xds: propagate call options to HTTP filter interceptor's NewStream.~~ xds/resolver: remove onCommitted() from RPCConfig. Jun 4, 2026

Pranjali-2501 assigned eshitachandwani and unassigned easwars Jun 4, 2026

Pranjali-2501 requested a review from eshitachandwani June 4, 2026 19:30

eshitachandwani reviewed Jun 5, 2026

View reviewed changes

Comment thread internal/xds/resolver/xds_resolver.go Outdated

eshitachandwani assigned Pranjali-2501 and unassigned eshitachandwani Jun 5, 2026

resolve comments

1534e5c

Pranjali-2501 requested a review from eshitachandwani June 5, 2026 08:55

Pranjali-2501 requested a review from eshitachandwani July 10, 2026 07:14

eshitachandwani reviewed Jul 10, 2026

View reviewed changes

eshitachandwani approved these changes Jul 10, 2026

View reviewed changes

minor nits

8a11187

mbissa approved these changes Jul 10, 2026

View reviewed changes

gemini-code-assist Bot reviewed Jul 10, 2026

View reviewed changes

Comment thread test/xds/xds_resolver_e2e_test.go Outdated

minor test modification

7f62ecc

Pranjali-2501 unassigned eshitachandwani Jul 10, 2026

easwars modified the milestones: 1.83 Release, 1.84 Release Jul 14, 2026

easwars reviewed Jul 14, 2026

View reviewed changes

easwars reviewed Jul 15, 2026

View reviewed changes

Comment thread test/xds/xds_resolver_e2e_test.go Outdated

easwars reviewed Jul 15, 2026

View reviewed changes

Comment thread test/xds/xds_resolver_e2e_test.go Outdated

easwars assigned Pranjali-2501 and unassigned easwars Jul 15, 2026

resolve comments

d855f2e

Pranjali-2501 requested a review from easwars July 27, 2026 13:21

Pranjali-2501 assigned easwars and unassigned Pranjali-2501 Jul 27, 2026

easwars approved these changes Jul 27, 2026

View reviewed changes

easwars assigned Pranjali-2501 and unassigned easwars Jul 27, 2026

minor change

654109c

sushanb mentioned this pull request Aug 10, 2026

xds/resolver: drop field references on Close to prevent retention across ClientConn recycle #9301

Merged

4 tasks

This was referenced Aug 11, 2026

Flaky Test : Test/StreamTimeoutBodyPhaseAllow #9305

Closed

extproc: increase timeout for TestStreamTimeoutBodyPhaseAllow and remove incorrect refcount decrement #9306

Merged

Uh oh!

Conversation

Pranjali-2501 commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Changes

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

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!

eshitachandwani left a comment

Choose a reason for hiding this comment

Uh oh!

mbissa left a comment

Choose a reason for hiding this comment

Uh oh!

Pranjali-2501 commented Jul 10, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

easwars commented Jul 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Pranjali-2501 commented May 25, 2026 •

edited

Loading

codecov Bot commented May 25, 2026 •

edited

Loading