Skip to content

core: exclude client and hedging cancellations from callcounters for outlier-detection - #12923

Open
AgraVator wants to merge 9 commits into
grpc:masterfrom
AgraVator:outlier-detection-hedging-cancellations
Open

core: exclude client and hedging cancellations from callcounters for outlier-detection#12923
AgraVator wants to merge 9 commits into
grpc:masterfrom
AgraVator:outlier-detection-hedging-cancellations

Conversation

@AgraVator

Copy link
Copy Markdown
Contributor

fixes #12834

@AgraVator AgraVator changed the title outlier-detection: exclude client and hedging cancellations from callcounters core: exclude client and hedging cancellations from callcounters for outlier-detection Jul 24, 2026
@AgraVator
AgraVator marked this pull request as ready for review July 27, 2026 08:58
@AgraVator
AgraVator requested a review from kannanjgithub July 27, 2026 08:58
Status status, RpcProgress rpcProgress, Metadata trailers) {
if (!listenerClosed) {
listenerClosed = true;
if (status.getCode() == Status.Code.CANCELLED) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is for transport callback for stream closure and it is too late at this point to make a distinction whether the cancellation is app/retriable stream initiated or from the remote endpoint. This should instead be done in method cancel that sets this.cancelled = true; and that call only happens for app/retriable stream initiated cancellation, and not for remote server initiated cancellations, and happens before the transport stream is closed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition, we would need the guard in AbstractClientStream.cancel against late cancellations:

     @Override
     public final void cancel(Status reason) {
       Preconditions.checkArgument(!reason.isOk(), "Should not cancel with OK status");
       
       if (cancelled || transportState().listenerClosed) {   //  <----- GUARD 
         return;
       }
       
      cancelled = true;
      statsTraceCtx.clientCancelled(reason);
      abstractClientStreamSink().cancel(reason);
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Outlier detection behavior for client cancellations and hedged streams

2 participants