IMPALA-7565: Add startup flag to set thrift connection setup thread pool size
This patch adds a hidden experimental startup flag called 'accepted_cnxn_setup_thread_pool_size' which enables setting the size of the thread pool used to process the internal post-accept, pre-setup connection queue in each thrift server set up to service Impala internal and external connections. The default is set to 1 which ensures that this change does not affect current behavior. Testing: Tested manually by adding a sleep and making sure other threads are used. Ran exhaustive tests with a pool size set to 10 successfully. Change-Id: I31344321a5f9e840a399ccb0f963c0759e2ab234 Reviewed-on: http://gerrit.cloudera.org:8080/11873 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/0a7da0fc Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/0a7da0fc Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/0a7da0fc Branch: refs/heads/branch-3.1.0 Commit: 0a7da0fc749aea0b73daa4ca12b1135b592a4de8 Parents: cb9db53 Author: Bikramjeet Vig <[email protected]> Authored: Fri Nov 2 15:36:17 2018 -0700 Committer: Zoltan Borok-Nagy <[email protected]> Committed: Tue Nov 13 12:51:39 2018 +0100 ---------------------------------------------------------------------- be/src/rpc/TAcceptQueueServer.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/0a7da0fc/be/src/rpc/TAcceptQueueServer.cpp ---------------------------------------------------------------------- diff --git a/be/src/rpc/TAcceptQueueServer.cpp b/be/src/rpc/TAcceptQueueServer.cpp index 0f89a7e..730901b 100644 --- a/be/src/rpc/TAcceptQueueServer.cpp +++ b/be/src/rpc/TAcceptQueueServer.cpp @@ -26,8 +26,14 @@ #include "util/thread-pool.h" DEFINE_int32(accepted_cnxn_queue_depth, 10000, - "(Advanced) The size of the post-accept, pre-setup connection queue for Impala " - "internal connections"); + "(Advanced) The size of the post-accept, pre-setup connection queue in each thrift " + "server set up to service Impala internal and external connections."); + +DEFINE_int32_hidden(accepted_cnxn_setup_thread_pool_size, 1, + "(Advanced) The size of the thread pool that is used to process the " + "post-accept, pre-setup connection queue in each thrift server set up to service " + "Impala internal and external connections. Warning: This is untested for values " + "greater than 1 which might exhibit unpredictable behavior and/or cause crashes."); namespace apache { namespace thrift { @@ -209,13 +215,17 @@ void TAcceptQueueServer::serve() { eventHandler_->preServe(); } - // Only using one thread here is sufficient for performance, and it avoids potential - // thread safety issues with the thrift code called in SetupConnection. - constexpr int CONNECTION_SETUP_POOL_SIZE = 1; - + if (FLAGS_accepted_cnxn_setup_thread_pool_size > 1) { + LOG(WARNING) << "connection_setup_thread_pool_size is set to " + << FLAGS_accepted_cnxn_setup_thread_pool_size + << ". Values greater than 1 are untested and might exhibit " + "unpredictable behavior and/or cause crashes."; + } // New - this is the thread pool used to process the internal accept queue. + // TODO: IMPALA-7565: Make sure the related thrift code is thread safe and subsequently + // enable multi-threading by default. ThreadPool<shared_ptr<TTransport>> connection_setup_pool("setup-server", "setup-worker", - CONNECTION_SETUP_POOL_SIZE, FLAGS_accepted_cnxn_queue_depth, + FLAGS_accepted_cnxn_setup_thread_pool_size, FLAGS_accepted_cnxn_queue_depth, [this](int tid, const shared_ptr<TTransport>& item) { this->SetupConnection(item); });
