xds: Implement separate request and response draining behavior in ext_proc client interceptor - #13011
Open
kannanjgithub wants to merge 9 commits into
Open
xds: Implement separate request and response draining behavior in ext_proc client interceptor#13011kannanjgithub wants to merge 9 commits into
kannanjgithub wants to merge 9 commits into
Conversation
When activateCall is called during draining (initiated by response drain), it flushes requests buffered in DelayedClientCall directly to the rawCall, bypassing flow control checks. To fix this: 1. Buffer request(n) calls in pendingRequests when the call is IDLE, instead of letting them go to DelayedClientCall. 2. Guard drainPendingRequests to check isResponseSidecarReady and abort if it is false (keeping requests buffered). 3. Trigger drainPendingRequests when response drain completes. Also updated tests to verify early request buffering during response drain. TAG=agy CONV=2c1e4760-c239-4698-810a-162bf10fccc4
Extend givenResponseDrainActive_whenAppRequestsMessages_thenRequestsBuffered to complete the response draining handshake and verify that buffered requests are then successfully sent to the data plane. TAG=agy CONV=2c1e4760-c239-4698-810a-162bf10fccc4
Add givenRequestDrainActive_whenAppRequestsMessages_thenRequestsDrained to verify that early request(n) calls are buffered when IDLE, and successfully drained to the data plane server upon activation if only request draining is active (which should not block the response/read path). TAG=agy CONV=2c1e4760-c239-4698-810a-162bf10fccc4
1. Fix onReadyNotify in DataPlaneListener to only forward onReady if dataPlaneClientCall.isReady() is true. 2. Call onReadyNotify in handleRequestDrainComplete to notify the app when request draining completes and the path becomes ready again. 3. Update givenRequestDrainActive_whenAppRequestsMessages_thenRequestsDrained to verify that onReady is NOT called during request drain but IS called after the drain completes. TAG=agy CONV=2c1e4760-c239-4698-810a-162bf10fccc4
1. Remove requestDrainComplete check from sendMessage to prevent out-of-order message delivery when app calls sendMessage concurrently with drain completion. Now we only bypass queue when passThroughMode is true. 2. Update givenExtProcStreamCompleted_whenIsReadyCalled_thenDelegatesToSuper test to expect the correct number of downstream isReady calls, which increased by 1 because we now correctly query isReady inside onReadyNotify on stream completion. TAG=agy CONV=2c1e4760-c239-4698-810a-162bf10fccc4
Remove requestDrainComplete check from immediate halfClose path, and safely restore early halfClose optimization under lock: allow immediate halfClose during requestDraining/requestDrainComplete only if body mode is NONE or if the buffer queue is empty and we either completed the handshake or haven't sent any body messages yet. TAG=agy CONV=2c1e4760-c239-4698-810a-162bf10fccc4
Initialize closeNow to observabilityMode to ensure the call is closed immediately when closing in observability mode, preventing hangs and avoiding duplicate proceedWithClose calls. Update givenObservabilityMode_whenDataPlaneClosed_thenSidecarCloseIsDeferred to assert that onClose is called exactly once, preventing double onClose notifications from passing silently. TAG=agy CONV=2c1e4760-c239-4698-810a-162bf10fccc4
Wrap long lines and fix empty/whitespace catch block in test to conform to style guidelines. TAG=agy CONV=2c1e4760-c239-4698-810a-162bf10fccc4
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
1 participant
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.
Implement separate request and response draining behavior in ext_proc client interceptor as per the updates in the gRFC.