ext_proc: closing the gRPC stream ASAP once no more external processing needed #41425

yanjunxiang-google · 2025-10-08T16:33:26Z

This is to address the issue: #41764, i.e, close the gRPC stream once no further external processing needed. It is also partially discussed in #37088.

Currently the ext_proc gRPC stream is opened when the 1st ProcessingRequest is sent to the ext_proc server. And it is closed during ext_proc filter destruction. This is wasting resource on both Envoy and ext_proc server side. For example, if envoy is configured to only send request headers, the gRPC stream is left open until all the way to the response is processed.

This PR is trying to close the ext_proc gRPC stream once Envoy detects no more external processing needed.

needed Signed-off-by: Yanjun Xiang <[email protected]>

Signed-off-by: Yanjun Xiang <[email protected]>

yanjunxiang-google · 2025-10-08T18:32:06Z

/retest

yanjunxiang-google · 2025-10-08T20:05:49Z

/assign @yanavlasov @tyxia @stevenzzzz

Signed-off-by: Yanjun Xiang <[email protected]>

yanavlasov

/wait

source/extensions/filters/http/ext_proc/ext_proc.cc

Signed-off-by: Yanjun Xiang <[email protected]>

stevenzzzz

Hooah. You are fighting a beast here. :)

high level, since this is a behavior change that impacts prod traffic, could you guard this change with a feature flag?

stevenzzzz

left some comments, pls consider some common cases like(there for sure are more):

CONTINUE_AND_REPLACE finishes one direction's all events.
EOS from trailers implicitly terminates body.
only check if-last-reponse after response is "processed", on error cases, stream will already be closed.

On a high level, if we have gone this far already, let's also consider to wait-for-trailers after the half close been sent. but that could be in another PR I assume.

source/extensions/filters/http/ext_proc/ext_proc.cc

source/extensions/filters/http/ext_proc/ext_proc.h

source/extensions/filters/http/ext_proc/ext_proc.cc

stevenzzzz · 2025-10-10T15:24:03Z

source/extensions/filters/http/ext_proc/ext_proc.cc

+    }
+    break;
+  case ProcessingResponse::ResponseCase::kRequestBody:
+    if (isLastBodyResponse(decoding_state_, *response) && encoding_state_.noExternalProcess()) {


what if trailers are configured?

Here we detect EoS is received with body, so no trailers in this request. Thus even filter is configured to send trailers, the external processing in this direction is completed.

Signed-off-by: Yanjun Xiang <[email protected]>

botengyao · 2025-10-14T12:21:30Z

Waiting for addressing comments.

/wait

Signed-off-by: Yanjun Xiang <[email protected]>

repokitteh-read-only · 2025-10-14T21:36:03Z

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #41425 was synchronize by yanjunxiang-google.

see: more, trace.

Signed-off-by: Yanjun Xiang <[email protected]>

tyxia

Thanks for working on this!

+1 on runtime guard to protect this change,. The processing mode/ext_proc is getting more and more complicated )

source/extensions/filters/http/ext_proc/ext_proc.cc

Signed-off-by: Yanjun Xiang <[email protected]>

yanjunxiang-google · 2025-10-17T14:23:42Z

The initial optimization does not apply for a few scenarios listed below, which is TBD in the future:

For BUFFERED and BUFFERED_PARTIAL body mode
During header response processing, if request does not have body but has trailer, and trailer is received, at same time if body sending mode is not NONE, and trailer sending mode is SKIP(unlikely request pattern, corner case)
During header response processing, if request has body and end_of_stream is received with body, at same time if body sending mode is NONE, and trailer sending mode is SEND(unlikely processing mode configuration, corner case).

yanjunxiang-google · 2025-10-20T20:06:59Z

Kind Ping!

Signed-off-by: Yanjun Xiang <[email protected]>

yanjunxiang-google · 2025-10-21T18:21:00Z

/retest

stevenzzzz

Thanks for working on this!

source/extensions/filters/http/ext_proc/ext_proc.cc

test/extensions/filters/http/ext_proc/ext_proc_full_duplex_integration_test.cc

Signed-off-by: Yanjun Xiang <[email protected]>

yanjunxiang-google · 2025-10-29T00:29:26Z

/retest

yanjunxiang-google · 2025-10-29T01:31:25Z

/retest

…_stream_asap Signed-off-by: Yanjun Xiang <[email protected]>

yanjunxiang-google · 2025-10-29T14:32:14Z

/retest

tyxia · 2025-10-29T14:57:53Z

@yanjunxiang-google Could you please resolve any conversations that you think is already resolved? It will help review.

Thanks!

yanjunxiang-google · 2025-10-29T16:22:56Z

@yanjunxiang-google Could you please resolve any conversations that you think is already resolved? It will help review.

Thanks!

Done, thanks!

…_stream_asap Signed-off-by: Yanjun Xiang <[email protected]>

yanjunxiang-google · 2025-10-29T17:33:59Z

/retest

yanjunxiang-google · 2025-10-29T18:57:03Z

/retest

yanjunxiang-google · 2025-10-30T14:03:55Z

/retest

…_stream_asap Signed-off-by: Yanjun Xiang <[email protected]>

yanavlasov · 2025-10-31T20:20:21Z

LGTM from me. Will wait for @tyxia and @stevenzzzz reviews.

tyxia

LGTM, Thanks

I think this PR is a best-effort attempt to close the gRPC stream ASAP, with trade-off of corner case complexity. It will be good to also run it with some load test etc.

yanjunxiang-google · 2025-11-01T14:37:12Z

/retest

ext_proc: closing the gRPC stream ASAP once no more external processing

755e111

needed Signed-off-by: Yanjun Xiang <[email protected]>

yanjunxiang-google requested review from mattklein123, tyxia and yanavlasov as code owners October 8, 2025 16:33

fix format

f6b536b

Signed-off-by: Yanjun Xiang <[email protected]>

repokitteh-read-only bot assigned stevenzzzz, tyxia and yanavlasov Oct 8, 2025

yanjunxiang-google added 5 commits October 8, 2025 21:55

set processing_complete_ to true

f8eba26

Signed-off-by: Yanjun Xiang <[email protected]>

adding server close test

dcf5c6c

Signed-off-by: Yanjun Xiang <[email protected]>

skip onGrpcClose() if processing_complete_ true

07be305

Signed-off-by: Yanjun Xiang <[email protected]>

fix test

49fbe96

Signed-off-by: Yanjun Xiang <[email protected]>

fix format

33a1d74

Signed-off-by: Yanjun Xiang <[email protected]>

yanavlasov requested changes Oct 9, 2025

View reviewed changes

source/extensions/filters/http/ext_proc/ext_proc.cc Outdated Show resolved Hide resolved

source/extensions/filters/http/ext_proc/ext_proc.cc Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Oct 9, 2025

address comments

35ef112

Signed-off-by: Yanjun Xiang <[email protected]>

repokitteh-read-only bot removed the waiting label Oct 9, 2025

stevenzzzz reviewed Oct 10, 2025

View reviewed changes

fix test

6d4aa09

Signed-off-by: Yanjun Xiang <[email protected]>

repokitteh-read-only bot added the waiting label Oct 14, 2025

moving stream close to the end of onReceiveMessage()

921c40b

Signed-off-by: Yanjun Xiang <[email protected]>

repokitteh-read-only bot removed the waiting label Oct 14, 2025

yanjunxiang-google added 3 commits October 14, 2025 21:16

addressing comments

bf71363

Signed-off-by: Yanjun Xiang <[email protected]>

adding runtime protection

2e8ab46

Signed-off-by: Yanjun Xiang <[email protected]>

fix format

b8934d1

Signed-off-by: Yanjun Xiang <[email protected]>

yanjunxiang-google added 2 commits October 15, 2025 21:19

fix format

e713075

Signed-off-by: Yanjun Xiang <[email protected]>

fix format

1a55c23

Signed-off-by: Yanjun Xiang <[email protected]>

tyxia reviewed Oct 16, 2025

View reviewed changes

source/extensions/filters/http/ext_proc/ext_proc.cc Outdated Show resolved Hide resolved

source/extensions/filters/http/ext_proc/ext_proc.cc Outdated Show resolved Hide resolved

source/extensions/filters/http/ext_proc/ext_proc.cc Show resolved Hide resolved

address comments

826a6cc

Signed-off-by: Yanjun Xiang <[email protected]>

yanjunxiang-google added 2 commits October 21, 2025 16:50

merge upstream main

422ec49

Signed-off-by: Yanjun Xiang <[email protected]>

moving stream closing to the end of encodeHeaders()

713ca86

Signed-off-by: Yanjun Xiang <[email protected]>

stevenzzzz reviewed Oct 22, 2025

View reviewed changes

yanjunxiang-google added 5 commits October 24, 2025 18:00

verifying stream is closed as expected with CONTINUE_AND_REPLACE

d72caae

Signed-off-by: Yanjun Xiang <[email protected]>

update changelogs and address comments

ac44492

Signed-off-by: Yanjun Xiang <[email protected]>

address comments

ac0233f

Signed-off-by: Yanjun Xiang <[email protected]>

fix format

23a6c0f

Signed-off-by: Yanjun Xiang <[email protected]>

fixing tests when graceful close is enabled

d40a260

Signed-off-by: Yanjun Xiang <[email protected]>

Merge branch 'main' of https://github.com/envoyproxy/envoy into close…

258aad1

…_stream_asap Signed-off-by: Yanjun Xiang <[email protected]>

yanjunxiang-google mentioned this pull request Oct 29, 2025

ext_proc: close the side gRPC stream once Envoy identify the external processing is completed #41764

Open

Merge branch 'main' of https://github.com/envoyproxy/envoy into close…

9a6fa7f

…_stream_asap Signed-off-by: Yanjun Xiang <[email protected]>

Merge branch 'main' of https://github.com/envoyproxy/envoy into close…

86ea4b2

…_stream_asap Signed-off-by: Yanjun Xiang <[email protected]>

tyxia approved these changes Oct 31, 2025

View reviewed changes

ext_proc: closing the gRPC stream ASAP once no more external processing needed #41425

Are you sure you want to change the base?

ext_proc: closing the gRPC stream ASAP once no more external processing needed #41425

Uh oh!

Conversation

yanjunxiang-google commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yanjunxiang-google commented Oct 8, 2025

Uh oh!

yanjunxiang-google commented Oct 8, 2025

Uh oh!

yanavlasov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stevenzzzz left a comment

Choose a reason for hiding this comment

Uh oh!

stevenzzzz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevenzzzz Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

yanjunxiang-google Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

botengyao commented Oct 14, 2025

Uh oh!

repokitteh-read-only bot commented Oct 14, 2025

Uh oh!

tyxia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yanjunxiang-google commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yanjunxiang-google commented Oct 20, 2025

Uh oh!

yanjunxiang-google commented Oct 21, 2025

Uh oh!

stevenzzzz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yanjunxiang-google commented Oct 29, 2025

Uh oh!

yanjunxiang-google commented Oct 29, 2025

Uh oh!

yanjunxiang-google commented Oct 29, 2025

Uh oh!

tyxia commented Oct 29, 2025

Uh oh!

yanjunxiang-google commented Oct 29, 2025

Uh oh!

yanjunxiang-google commented Oct 29, 2025

Uh oh!

yanjunxiang-google commented Oct 29, 2025

Uh oh!

yanjunxiang-google commented Oct 30, 2025

yanjunxiang-google commented Oct 8, 2025 •

edited

Loading

yanjunxiang-google commented Oct 17, 2025 •

edited

Loading

tyxia left a comment •

edited

Loading