[jira] [Created] (IMPALA-7565) Extends TAcceptQueueServer connection_setup_pool to be multi-threaded

["Michael Ho (JIRA)" <ji...@apache.org>]

Michael Ho created IMPALA-7565:
----------------------------------

             Summary: Extends TAcceptQueueServer connection_setup_pool to be multi-threaded
                 Key: IMPALA-7565
                 URL: https://issues.apache.org/jira/browse/IMPALA-7565
             Project: IMPALA
          Issue Type: Improvement
          Components: Clients, Distributed Exec
    Affects Versions: Impala 2.12.0, Impala 3.0, Impala 2.11.0
            Reporter: Michael Ho


In {{TAcceptQueueServer.cpp}}, we currently have one thread in the {{connection_setup_pool}} for handling connection establishment. 

{noformat}
  // 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;

  // New - this is the thread pool used to process the internal accept queue.
  ThreadPool<shared_ptr<TTransport>> connection_setup_pool("setup-server", "setup-worker",
      CONNECTION_SETUP_POOL_SIZE, FLAGS_accepted_cnxn_queue_depth,
      [this](int tid, const shared_ptr<TTransport>& item) {
        this->SetupConnection(item);
      });
{noformat}

While that makes the code easier to reason about, it also makes it more vulnerable to denial-of-service attack in case a client intentionally or unintentionally holds up the connection establishment. For instance, one can telnet to the beeswax or HS2 port of Impala (e.g. telnet localhost:21000) and then leave the telnet session open. In a secure cluster with TLS enabled, Impalad will be stuck in the SSL handshake. Other clients trying to connect to that port (e.g. Impala shell) will hang forever.

{noformat}
Thread 551 (Thread 0x7fddde563700 (LWP 166354)):
#0  0x0000003ce2a0e82d in read () from /lib64/libpthread.so.0
#1  0x0000003ce56dea71 in ?? () from /usr/lib64/libcrypto.so.10
#2  0x0000003ce56dcdc9 in BIO_read () from /usr/lib64/libcrypto.so.10
#3  0x0000003ce9a31873 in ssl23_read_bytes () from /usr/lib64/libssl.so.10
#4  0x0000003ce9a2fe63 in ssl23_get_client_hello () from /usr/lib64/libssl.so.10
#5  0x0000003ce9a302f3 in ssl23_accept () from /usr/lib64/libssl.so.10
#6  0x000000000208ebd5 in apache::thrift::transport::TSSLSocket::checkHandshake() ()
#7  0x000000000208edbc in apache::thrift::transport::TSSLSocket::read(unsigned char*, unsigned int) ()
#8  0x000000000208b6f3 in unsigned int apache::thrift::transport::readAll<apache::thrift::transport::TSocket>(apache::thrift::transport::TSocket&, unsigned char*, unsigned int) ()
#9  0x0000000000cb2aa9 in apache::thrift::transport::TSaslTransport::receiveSaslMessage(apache::thrift::transport::NegotiationStatus*, unsigned int*) ()
#10 0x0000000000cb03e4 in apache::thrift::transport::TSaslServerTransport::handleSaslStartMessage() ()
#11 0x0000000000cb2c23 in apache::thrift::transport::TSaslTransport::doSaslNegotiation() ()
#12 0x0000000000cb10b8 in apache::thrift::transport::TSaslServerTransport::Factory::getTransport(boost::shared_ptr<apache::thrift::transport::TTransport>) ()
#13 0x0000000000b13e47 in apache::thrift::server::TAcceptQueueServer::SetupConnection(boost::shared_ptr<apache::thrift::transport::TTransport>) ()
#14 0x0000000000b14932 in boost::detail::function::void_function_obj_invoker2<apache::thrift::server::TAcceptQueueServer::serve()::{lambda(int, boost::shared_ptr<apache::thrift::transport::TTransport> const&)#1}, void, int, boost::shared_ptr<apache::thrift::transport::TTransport> const&>::invoke(boost::detail::function::function_buffer&, int, boost::shared_ptr<apache::thrift::transport::TTransport> const&) ()
#15 0x0000000000b177f9 in impala::ThreadPool<boost::shared_ptr<apache::thrift::transport::TTransport> >::WorkerThread(int) ()
#16 0x0000000000d602af in impala::Thread::SuperviseThread(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, boost::function<void ()()>, impala::ThreadDebugInfo const*, impala::Promise<long>*) ()
#17 0x0000000000d60aaa in boost::detail::thread_data<boost::_bi::bind_t<void, void (*)(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, boost::function<void ()()>, impala::ThreadDebugInfo const*, impala::Promise<long>*), boost::_bi::list5<boost::_bi::value<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, boost::_bi::value<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, boost::_bi::value<boost::function<void ()()> >, boost::_bi::value<impala::ThreadDebugInfo*>, boost::_bi::value<impala::Promise<long>*> > > >::run() ()
#18 0x00000000012d756a in thread_proxy ()
#19 0x0000003ce2a07aa1 in start_thread () from /lib64/libpthread.so.0
#20 0x0000003ce26e893d in clone () from /lib64/libc.so.6
{noformat}

While it's not a complete fix, increasing the number of threads in {{connection_setup_pool}} makes it more robust against this kind of problem. Ideally, the number of threads in the thread pool should be configurable.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscribe@impala.apache.org
For additional commands, e-mail: issues-all-help@impala.apache.org