impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joe McDonnell (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5750: Catch exceptions from boost thread creation
Date Fri, 18 Aug 2017 22:44:01 GMT
Joe McDonnell has uploaded a new patch set (#2).

Change subject: IMPALA-5750: Catch exceptions from boost thread creation
......................................................................

IMPALA-5750: Catch exceptions from boost thread creation

The boost thread constructor will throw
boost::thread_resource_error if it is unable to spawn
a thread on the system (e.g. due to a ulimit). This
uncaught exception crashes Impala. Systems with a large
number of nodes and threads are hitting this limit.

This change catches the exception from the thread
constructor and converts it to a Status. This requires
several changes:
1. util/thread.h's Thread constructor is now private
and all Threads are constructed via a new Create()
static factory method.
2. util/thread-pool.h's ThreadPool constructor is now
private and all ThreadPools are constructed via a
new Create() static factory method.
3. To propagate the Status, Threads and ThreadPools
cannot be created in constructors, so this is moved to
initialization methods that can return Status.
4. Since Threads and ThreadPools need to be passed
as arguments to the Create method, this eliminates
the ability to use them as stack-allocated local
variables or direct declarations in classes. These
have been replaced with scoped_ptr's.

Remaining TODO:
1. QueryState::StartFInstances() needs appropriate
cleanup if thread create fails. This is intricate.
2. There is some duplicate code between ThreadPool
and CallableThreadPool to try to eliminate.
3. The Thread::Create and ThreadPool::Create have
both pointer and scoped_ptr versions. It would be
nice to only have one version.

Change-Id: I15a2f278dc71892b7fec09593f81b1a57ab725c0
---
M be/src/benchmarks/thread-create-benchmark.cc
M be/src/catalog/catalog-server.cc
M be/src/common/init.cc
M be/src/common/init.h
M be/src/exec/blocking-join-node.cc
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/kudu-scan-node.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-thread.cc
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/disk-io-mgr.cc
M be/src/runtime/exec-env.cc
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/parallel-executor.cc
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/scheduling/admission-controller.cc
M be/src/service/child-query.cc
M be/src/service/child-query.h
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/statestore/statestored-main.cc
M be/src/testutil/in-process-servers.cc
M be/src/util/hdfs-bulk-ops.cc
M be/src/util/hdfs-bulk-ops.h
M be/src/util/thread-pool-test.cc
M be/src/util/thread-pool.h
M be/src/util/thread.cc
M be/src/util/thread.h
38 files changed, 371 insertions(+), 177 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/7730/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I15a2f278dc71892b7fec09593f81b1a57ab725c0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <joemcdonnell@cloudera.com>

Mime
View raw message