Repository navigation
core: Reset configSelector on realChannel while entering IDLE state - #12832
Conversation
|
|
|
Hey, |
There was a problem hiding this comment.
Pull request overview
Fixes a ManagedChannel idle-mode edge case where a stale InternalConfigSelector can prevent the channel from properly exiting IDLE after xDS resources disappear and later reappear.
Changes:
- Reset
realChannel’s config selector toINITIAL_PENDING_SELECTORwhen entering IDLE, forcing the next call to trigger resolver/LB restart. - Prevent pending-call reprocessing when
updateConfigSelector()is called withINITIAL_PENDING_SELECTOR(only reprocess when transitioning away from the initial pending selector).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The tests are passing locally. Also the failures in tests(11) and tests(17) runs are unrelated to changes I made. And same tests passed in tests(8) and test(21). Also I don't see an option to retrigger the failed tests. |
|
Is it anything else I should do to get the change reviewed ? We face this issue occasionally in production and it requires a service restart to reset GRPC channel |
Can you address the Copilot review comment above for adding a new test? |
I added tests. Please note, I used Copilot to navigate and create the tests |
|
Failed test(11) is not related to my changes. Same test passed for other Java versions |
Is there anything I should do to get this merged ? |
|
@AgraVator Could you please take a look at the change ? |
Hey @kkalin68 feel free to merge as @kannanjgithub has already approved the changes |
|
@AgraVator @kannanjgithub GH tells me I cannot merge since PR needs 2 reviewers and only one is approved. Or may be the issue is in failed build for Java 11. |
Resolves one conflict in ManagedChannelImpl.updateConfigSelector(). Upstream grpc#12832 resets the config selector when the channel enters IDLE, by calling realChannel.updateConfigSelector(INITIAL_PENDING_SELECTOR) from enterIdleMode(), and guards the method so that this reset does not reprocess the queued calls. This branch had rewritten the same method to release the pending calls through releasePendingCalls() and no longer has the prevConfig check that the guard was added to, so the two changes could not be combined textually. The reset is now handled by an early return that keeps upstream's behaviour: the new config selector is stored, but nothing is released. It also clears initialConfigResolved and lastResolutionError, because entering IDLE shuts the name resolver down and the channel really is back to the state it was in before its first resolution. Leaving initialConfigResolved set would make a resolution failure after the idle period skip the default service config in onConfigError(), and a stale lastResolutionError would be reported as the delay reason for a call queued after the channel went idle. Everything else merged cleanly.
ManagedChannel will be stuck in IDLE state when xDS control plane doesn't have a resource anymore.
The scenario is following: