This is an automated email from the ASF dual-hosted git repository. michaelsmith pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 81844499b51da092567c510202a4b7de81ecd8af Author: Joe McDonnell <[email protected]> AuthorDate: Tue Aug 22 10:36:12 2023 -0700 IMPALA-12366: Use 2GB as the default for thrift_rpc_max_message_size Thrift 0.16 implemented a limit on the max message size. In IMPALA-11669, we added the thrift_rpc_max_message_size parameter and set the default size to 1GB. Some existing clusters have needed to tune this parameter higher because their workloads use message sizes larger than 1GB (e.g. for metadata updates). Historically, Impala has been able to send and receive 2GB messages, so this changes the default value for thrift_rpc_max_message_size to 2GB (INT_MAX). This can be reduced in future when Impala can guarantee that messages work properly when split up into smaller batches. TestGracefulShutdown::test_shutdown_idle started failing with this change, because it is producing a different error message for one of the negative tests. ClientRequestState::ExecShutdownRequest() appends some extra explanation when it sees a "Network error" KRPC error, and the test expects that extra explanation. This modifies ClientRequestState::ExecShutdownRequest() to provide the extra explanation for the new error ("Timed out") as well. Testing: - Ran GVO Change-Id: Ib624201b683966a9feefb8fe45985f3d52d869fc Reviewed-on: http://gerrit.cloudera.org:8080/20394 Tested-by: Impala Public Jenkins <[email protected]> Reviewed-by: Riza Suminto <[email protected]> Reviewed-by: Michael Smith <[email protected]> --- be/src/rpc/thrift-util.cc | 10 ++++++---- be/src/service/client-request-state.cc | 5 ++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/be/src/rpc/thrift-util.cc b/be/src/rpc/thrift-util.cc index 502b7c22b..ce88b9b9b 100644 --- a/be/src/rpc/thrift-util.cc +++ b/be/src/rpc/thrift-util.cc @@ -17,6 +17,8 @@ #include "rpc/thrift-util.h" +#include <limits> + #include <gtest/gtest.h> #include <thrift/config.h> @@ -56,10 +58,10 @@ #include "common/names.h" -DEFINE_int32(thrift_rpc_max_message_size, (1024 * 1024 * 1024), - "The maximum size of a message that any RPC that the server will accept. " - "Default to 1GB. Setting 0 or negative value will use the default defined in the " - "Thrift. The upper limit is 2147483647 bytes."); +DEFINE_int32(thrift_rpc_max_message_size, std::numeric_limits<int32_t>::max(), + "The maximum size of a message for any RPC that the server will accept. " + "Default to the upper limit of 2147483647 bytes (~2GB). " + "Setting 0 or negative value will use the default defined in Thrift."); using namespace apache::thrift; using namespace apache::thrift::transport; diff --git a/be/src/service/client-request-state.cc b/be/src/service/client-request-state.cc index f6c0fbb2b..0f9764201 100644 --- a/be/src/service/client-request-state.cc +++ b/be/src/service/client-request-state.cc @@ -983,6 +983,7 @@ Status ClientRequestState::ExecShutdownRequest() { } } string krpc_error = "RemoteShutdown() RPC failed: Network error"; + string krpc_error2 = "RemoteShutdown() RPC failed: Timed out"; NetworkAddressPB krpc_addr = MakeNetworkAddressPB(ip_address, port, backend_id, ExecEnv::GetInstance()->rpc_mgr()->GetUdsAddressUniqueId()); std::unique_ptr<ControlServiceProxy> proxy; @@ -1014,7 +1015,9 @@ Status ClientRequestState::ExecShutdownRequest() { string err_string = Substitute( "Rpc to $0 failed with error '$1'", NetworkAddressPBToString(krpc_addr), msg); // Attempt to detect if the the failure is because of not using a KRPC port. - if (backend_port_specified && msg.find(krpc_error) != string::npos) { + if (backend_port_specified && + (msg.find(krpc_error) != string::npos || + msg.find(krpc_error2) != string::npos)) { // Prior to IMPALA-7985 :shutdown() used the backend port. err_string.append(" This may be because the port specified is wrong. You may have" " specified the backend (thrift) port which :shutdown() can no"
