Skip to content

thrift: don't close the downstream on an upstream overflow #19133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zuercher merged 5 commits into from
Dec 2, 2021
Merged

thrift: don't close the downstream on an upstream overflow #19133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
22fc092
thrift: don't close the downstream connection on upstream overflow er…
Nov 29, 2021
e13f8bb
Fix ordering
Nov 30, 2021
cffc2c1
Fix test
Nov 30, 2021
d78484d
Merge remote-tracking branch 'upstream/main' into thrift-handle-upstr…
Dec 2, 2021
5e2dc5b
Fix changelog
Dec 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Bug Fixes
* ext_authz: fix the ext_authz http filter to correctly set response flags to ``UAEX`` when a connection is denied.
* ext_authz: fix the ext_authz network filter to correctly set response flag and code details to ``UAEX`` when a connection is denied.
* listener: fixed issue where more than one listener could listen on the same port if using reuse port, thus randomly accepting connections on different listeners. This configuration is now rejected.
* thrift_proxy: do not close downstream connections when an upstream connection overflow happens.
* thrift_proxy: fix the thrift_proxy connection manager to correctly report success/error response metrics when performing :ref:`payload passthrough <envoy_v3_api_field_extensions.filters.network.thrift_proxy.v3.ThriftProxy.payload_passthrough>`.

Removed Config or Runtime
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/filters/network/thrift_proxy/router/upstream_request.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ void UpstreamRequest::onResetStream(ConnectionPool::PoolFailureReason reason) {
stats_.incResponseLocalException(parent_.cluster());
parent_.sendLocalReply(AppException(AppExceptionType::InternalError,
"thrift upstream request: too many connections"),
true);
false /* Don't close the downstream connection. */);
break;
case ConnectionPool::PoolFailureReason::LocalConnectionFailure:
upstream_host_->outlierDetector().putResult(
Expand Down
2 changes: 1 addition & 1 deletion test/extensions/filters/network/thrift_proxy/router_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ TEST_F(ThriftRouterTest, PoolOverflowFailure) {
auto& app_ex = dynamic_cast<const AppException&>(response);
EXPECT_EQ(AppExceptionType::InternalError, app_ex.type_);
EXPECT_THAT(app_ex.what(), ContainsRegex(".*too many connections.*"));
EXPECT_TRUE(end_stream);
EXPECT_FALSE(end_stream);
}));
context_.cluster_manager_.thread_local_cluster_.tcp_conn_pool_.poolFailure(
ConnectionPool::PoolFailureReason::Overflow, true);
Expand Down