tcp proxy: add new option to let tcp proxy check the drain close status by wbpcode · Pull Request #44567 · envoyproxy/envoy

wbpcode · 2026-04-22T03:01:51Z

Commit Message: tcp proxy: add new option to let tcp proxy check the drain close status
Additional Description:

To close #44419.

There are some other solutions in my mind, like to register a callback for every connection to the drain close manager. But it will introduce huge complexity to ensure all these callbacks are called correctly in correct threads and have impact to our core code tree.

So, I finally select the simplest one. This is clean, safe, easy to review/maintain, and kept similar logic with HCM.

Risk Level: low. touch core code but guarded by new proto API.
Testing: unit.
Docs Changes: n/a.
Release Notes: added.
Platform Specific Features: n/a.

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

repokitteh-read-only · 2026-04-22T03:02:01Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #44567 was opened by wbpcode.

see: more, trace.

wbpcode · 2026-04-22T11:39:07Z

/retest

adisuissa · 2026-04-28T12:35:06Z

 * All tcp proxy stats. @see stats_macros.h
 */
 #define ALL_TCP_PROXY_STATS(COUNTER, GAUGE)                                                        \
+  COUNTER(downstream_cx_drain_close)                                                               \


Is this counter described in a doc?

Let's me take a check.

… dev-fix-tcp-proxy

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

agrawroh

Just one question, LGTM otherwise

… dev-fix-tcp-proxy

agrawroh · 2026-05-02T18:53:00Z

/retest

wbpcode · 2026-05-04T03:32:00Z

I guess the CI failure have no business with this PR...

… dev-fix-tcp-proxy

wbpcode · 2026-05-04T03:36:58Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a check_drain_close configuration option to the TCP proxy filter. When enabled, the filter checks for a drain signal after each read or write operation and closes the downstream connection using FlushWrite if a drain is requested. The changes include updates to the API, documentation, implementation in tcp_proxy.cc, and comprehensive unit tests. A review comment suggests that the drain check in onData should be extended to cover active proxying paths where an upstream connection exists, ensuring consistent behavior.

Copilot

Pull request overview

Adds an opt-in TCP proxy feature to proactively honor drain-close decisions during active TCP data flow, reducing the likelihood of reconnect storms at the end of listener drain.

Changes:

Add check_drain_close API field and wire it into TcpProxy::Filter to close downstream connections with FlushWrite when drain close is requested.
Add a new stat (downstream_cx_drain_close) and a new local close reason (tcp_proxy_drain_close) for observability.
Add/extend unit tests and update TCP proxy stats documentation.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/mocks/server/factory_context.h	Extend mock factory context with `MockListenerInfo` to support listener direction-dependent behavior.
test/mocks/server/factory_context.cc	Provide default `listenerInfo()`/`direction()` behavior for tests.
test/extensions/filters/network/tcp_proxy/config_test.cc	Add config parsing test for the new `check_drain_close` field.
test/common/tcp_proxy/tcp_proxy_test.cc	Add unit coverage for drain-close behavior on downstream read/upstream write and for inbound-only scope selection.
source/common/tcp_proxy/tcp_proxy.h	Add new stat and config accessors for drain-close decision/scope and the feature flag.
source/common/tcp_proxy/tcp_proxy.cc	Implement drain-close check and downstream closure after read/write handling when enabled.
envoy/stream_info/stream_info.h	Add `TcpProxyDrainClose` local close reason string.
docs/root/configuration/listeners/network_filters/tcp_proxy_filter.rst	Document the new downstream drain-close counter.
changelogs/current.yaml	Add release note entry describing the new TCP proxy drain-close check feature.
api/envoy/extensions/filters/network/tcp_proxy/v3/tcp_proxy.proto	Add `check_drain_close` field and update next-free-field annotation.

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

kyessenov · 2026-05-04T17:02:29Z

+  // the downstream connection is closed with ``FlushWrite``.
+  //
+  // This is disabled by default for backward compatibility.
+  bool check_drain_close = 24;


Curious: does HTTP manager do something similar? Does it poll the state of connections or does it register a callback with the drain manager? I feel like the polling method adds non-determinism (e.g. active connections are closed but idle connection remain stale), which makes it hard to use.

The HCM will poll the state of drain manager at the end of every single request. This also one reason why I choose the polling implementation.

I feel like the polling method adds non-determinism (e.g. active connections are closed but idle connection remain stale), which makes it hard to use.

Yeah. This is known problem/shortage. But callbacks solution will brings huge complexity to core code (callback lifetime management, thread safe, graceful draining and so on). I guess an appropriate idle timeout setting combine the polling should be good enough for most scenarios?

wbpcode · 2026-05-05T08:46:13Z

/retest

adisuissa

/lgtm api

ggreenway

Looks good overall; just a few nits

/wait

ggreenway · 2026-05-06T22:20:31Z

+  // after each read or write. When drain close is requested for the listener's traffic direction,
+  // the downstream connection is closed with ``FlushWrite``.
+  //
+  // This is disabled by default for backward compatibility.


Should we enable by default and leave a runtime setting to temporarily change the default. It seems like much better behavior to have this enabled.

I am not sure if there is a case that the users want to keep the existing TCP connections as long time as possible by setting a long drain duration. Different with HTTP, we have no way to close the connection gracefully.
So, I slightly inclined to keep an option to control it. But I can change the bool to wrapped value so it's would be easier to change the default value in the future. WDYT?

If you strongly prefer to use runtime guard and enable it by default, I am also fine to that. Feel free to let me know.

I don't feel strongly, it was just a thought. I'm ok leaving it as is.

If you used a message here, you could have put a timer-based check as well in the future. I don't think we see TCP keep-alives in Envoy, unlike HTTP, so a connection can be stuck without drain or data without a timer.

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

… dev-fix-tcp-proxy

wbpcode · 2026-05-08T08:43:43Z

This API is reviewed before. And latest one only change it to wrapped message.

…us (envoyproxy#44567) Commit Message: tcp proxy: add new option to let tcp proxy check the drain close status Additional Description: To close envoyproxy#44419. There are some other solutions in my mind, like to register a callback for every connection to the drain close manager. But it will introduce huge complexity to ensure all these callbacks are called correctly in correct threads and have impact to our core code tree. So, I finally select the simplest one. This is clean, safe, easy to review/maintain, and kept similar logic with HCM. Risk Level: low. touch core code but guarded by new proto API. Testing: unit. Docs Changes: n/a. Release Notes: added. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com> Signed-off-by: Jiawei Wu <wujiawei@google.com>

…us (envoyproxy#44567) Commit Message: tcp proxy: add new option to let tcp proxy check the drain close status Additional Description: To close envoyproxy#44419. There are some other solutions in my mind, like to register a callback for every connection to the drain close manager. But it will introduce huge complexity to ensure all these callbacks are called correctly in correct threads and have impact to our core code tree. So, I finally select the simplest one. This is clean, safe, easy to review/maintain, and kept similar logic with HCM. Risk Level: low. touch core code but guarded by new proto API. Testing: unit. Docs Changes: n/a. Release Notes: added. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com> Signed-off-by: Alireza <alrzazz98@gmail.com>

tcp proxy: add new option to let tcp proxy check the drain close status

cc90be1

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

wbpcode requested review from ggreenway and zuercher as code owners April 22, 2026 03:01

repokitteh-read-only Bot added the api label Apr 22, 2026

repokitteh-read-only Bot assigned adisuissa Apr 22, 2026

wbpcode assigned ggreenway and agrawroh Apr 22, 2026

adisuissa reviewed Apr 28, 2026

View reviewed changes

ravenblackx added the waiting:any label Apr 28, 2026

repokitteh-read-only Bot removed waiting:any labels Apr 29, 2026

wbpcode added 4 commits April 29, 2026 02:53

Merge branch 'main' of ssh://ssh.github.com:443/envoyproxy/envoy into…

b5d3597

… dev-fix-tcp-proxy

update comment

89681fb

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

add document of stats

801544d

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

fix format

e3fa7cb

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

agrawroh reviewed Apr 30, 2026

View reviewed changes

Comment thread source/common/tcp_proxy/tcp_proxy.cc

agrawroh reviewed Apr 30, 2026

View reviewed changes

Merge branch 'main' of ssh://ssh.github.com:443/envoyproxy/envoy into…

75bc67f

… dev-fix-tcp-proxy

agrawroh previously approved these changes May 2, 2026

View reviewed changes

Merge branch 'main' of ssh://ssh.github.com:443/envoyproxy/envoy into…

39b1259

… dev-fix-tcp-proxy

wbpcode requested a review from Copilot May 4, 2026 03:36

Copilot started reviewing on behalf of wbpcode May 4, 2026 03:37 View session

gemini-code-assist Bot reviewed May 4, 2026

View reviewed changes

Comment thread source/common/tcp_proxy/tcp_proxy.cc

Copilot AI reviewed May 4, 2026

View reviewed changes

Comment thread changelogs/current.yaml

try improve coverage

7f7c7e5

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

wbpcode dismissed agrawroh’s stale review via 7f7c7e5 May 4, 2026 14:34

wbpcode requested a review from yanavlasov as a code owner May 4, 2026 14:34

kyessenov reviewed May 4, 2026

View reviewed changes

adisuissa reviewed May 5, 2026

View reviewed changes

repokitteh-read-only Bot removed the api label May 5, 2026

agrawroh previously approved these changes May 6, 2026

View reviewed changes

ggreenway requested changes May 6, 2026

View reviewed changes

repokitteh-read-only Bot added the waiting label May 6, 2026

address some comments

07169e2

Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>

wbpcode dismissed agrawroh’s stale review via 07169e2 May 7, 2026 01:42

repokitteh-read-only Bot added api and removed waiting labels May 7, 2026

Merge branch 'main' of ssh://ssh.github.com:443/envoyproxy/envoy into…

ddd5f8e

… dev-fix-tcp-proxy

ggreenway approved these changes May 7, 2026

View reviewed changes

agrawroh approved these changes May 8, 2026

View reviewed changes

wbpcode merged commit 4a9081f into envoyproxy:main May 8, 2026
29 of 30 checks passed

wbpcode deleted the dev-fix-tcp-proxy branch May 8, 2026 08:44

wbpcode mentioned this pull request May 8, 2026

listener_manager: implement filter-chain drain-close callback #44932

Open

Conversation

wbpcode commented Apr 22, 2026

Uh oh!

repokitteh-read-only Bot commented Apr 22, 2026

Uh oh!

wbpcode commented Apr 22, 2026

Uh oh!

adisuissa Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

wbpcode Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

agrawroh left a comment

Choose a reason for hiding this comment

Uh oh!

agrawroh commented May 2, 2026

Uh oh!

wbpcode commented May 4, 2026

Uh oh!

wbpcode commented May 4, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

kyessenov May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wbpcode May 5, 2026

Choose a reason for hiding this comment

Uh oh!

wbpcode commented May 5, 2026

Uh oh!

adisuissa left a comment

Choose a reason for hiding this comment

Uh oh!

ggreenway left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ggreenway May 6, 2026

Choose a reason for hiding this comment

Uh oh!

wbpcode May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wbpcode May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggreenway May 7, 2026

Choose a reason for hiding this comment

Uh oh!

kyessenov May 7, 2026

Choose a reason for hiding this comment

Uh oh!

wbpcode commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

kyessenov May 4, 2026 •

edited

Loading

wbpcode May 7, 2026 •

edited

Loading

wbpcode May 7, 2026 •

edited

Loading

wbpcode commented May 8, 2026 •

edited

Loading