Repository navigation
gRPC client: release the call slot before the caller is notified - #1623
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The fix lacks the required changes.md release-note entry documenting the premature call-limit rejection and referencing issue #1622.
1 open issue
What changed in this PR
Fixes the gRPC client admission race behind issue #1622 by releasing the call slot before notifying the caller, preventing immediate follow-up calls from being spuriously rejected.
Changes:
- Moves slot release ahead of
super.deframerClosed(). - Adds a regression test alternating 200 failing and healthy calls with a single available slot.
| File | Description |
|---|---|
org.restlet.ext.grpc/src/test/java/org/restlet/ext/grpc/GrpcHttpValidationTestCase.java |
Adds repeated failure-and-recovery coverage for single-slot admission. |
org.restlet.ext.grpc/src/main/java/org/restlet/ext/grpc/internal/RestletClientStream.java |
Releases the call slot before caller notification. |
Review effort: Balanced
|
@copilot resolve the merge conflicts in this pull request |
Merged |
RestletClientStream released its admission permit in deframerClosed() only after super.deframerClosed() had returned, but the SDK notifies the call listener (and so wakes the caller) from that super call. A caller that started its next call right away could find the single permit still taken and get RESOURCE_EXHAUSTED "gRPC call limit exceeded". It made GrpcHttpValidationTestCase fail intermittently on CI (4 runs in a row). The permit is now released first. Added a regression test alternating 200 failing and healthy calls with one slot. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Co-authored-by: jlouvel <[email protected]>
b0f6b74 to
7d22240
Compare
|
The aim
Fixes #1622.
GrpcHttpValidationTestCasefails intermittently on CI (the last four runs on3.0), each time on a different test, withRESOURCE_EXHAUSTED: gRPC call limit exceededraised by the second call of the test.The solution
RestletClientStream.deframerClosed()released the admission permit only aftersuper.deframerClosed()had returned. The SDK notifies the call listener, and so wakes the blocked caller, from inside that super call (checked in thegrpc-corebytecode). A caller starting its next call right away could find the single permit still taken. The permit is now released first.Added
failedCallReleasesItsSlotBeforeTheCallerIsNotified, which alternates 200 failing and healthy calls with one slot.Not addressed: very rarely a failing call ends as
UNAVAILABLE(IOException: Stream closedwhile the worker reads the body) instead ofUNKNOWN. The call still fails and its slot is released; the new test does not depend on the status code.Reproduction
Before the fix,
taskset -c 0 ./mvnw -o -pl org.restlet.ext.grpc -am test -Dtest=GrpcHttpValidationTestCasefailed in 10 of 12 runs. After the fix, 15 of 15 runs pass, and the whole gRPC module passes (37 tests).Check-list
changes.mdentry: the gRPC client is experimental and its earlier fixes have none)🤖 Generated with Claude Code