impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joe McDonnell (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5750: Catch exceptions from boost thread creation
Date Fri, 25 Aug 2017 23:57:42 GMT
Joe McDonnell has uploaded a new patch set (#4).

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 requires that Init()
be called after the ThreadPool is constructed.
3. To propagate the Status, Threads cannot be created in
constructors, so this is moved to initialization methods
that can return Status.
4. Threads now use unique_ptr's for management in all
cases. Threads cannot be used as stack-allocated local
variables or direct declarations in classes.

Query execution code paths will now handle the error:
1. Non-MT scan nodes, such as HDFS scan node and Kudu
scan node require at least one scanner thread to
make progress. This patch will abort the query if
the scan node cannot obtain at least one scanner thread.
However, if the scan node has at least one scanner
thread, the query will continue.
2. Failing to spawn a fragment instance from the query
state in StartFInstances() will correctly report the error
to the coordinator and tear down the query.

Thread::Create() has an optional argument to randomly
inject thread creation errors at any single point in
the code. It causes roughly 1% of the calls to fail.
The code was tested by setting this option in different
code points and verifying that the queries either
run to completion with the correct result or fail with
the appropriate status.

Change-Id: I15a2f278dc71892b7fec09593f81b1a57ab725c0
M be/src/benchmarks/
M be/src/catalog/
M be/src/catalog/catalog-server.h
M be/src/catalog/
M be/src/common/
M be/src/common/init.h
M be/src/exec/
M be/src/exec/
M be/src/exec/hdfs-scan-node.h
M be/src/exec/
M be/src/exec/kudu-scan-node-base.h
M be/src/exec/
M be/src/exec/kudu-scan-node.h
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/auth-provider.h
M be/src/rpc/
M be/src/rpc/
M be/src/rpc/
M be/src/rpc/thrift-server.h
M be/src/rpc/
M be/src/rpc/thrift-thread.h
M be/src/runtime/
M be/src/runtime/disk-io-mgr-handle-cache.h
M be/src/runtime/disk-io-mgr-handle-cache.inline.h
M be/src/runtime/
M be/src/runtime/
M be/src/runtime/
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/
M be/src/runtime/
M be/src/runtime/
M be/src/runtime/query-state.h
M be/src/runtime/thread-resource-mgr.h
M be/src/scheduling/
M be/src/scheduling/admission-controller.h
M be/src/service/
M be/src/service/child-query.h
M be/src/service/
M be/src/service/client-request-state.h
M be/src/service/
M be/src/service/
M be/src/service/
M be/src/service/impala-server.h
M be/src/service/
M be/src/statestore/
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/
M be/src/statestore/statestore.h
M be/src/statestore/
M be/src/testutil/
M be/src/testutil/in-process-servers.h
M be/src/util/
M be/src/util/thread-pool.h
M be/src/util/
M be/src/util/thread.h
M common/thrift/
56 files changed, 444 insertions(+), 225 deletions(-)

  git pull ssh:// refs/changes/30/7730/4
To view, visit
To unsubscribe, visit

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I15a2f278dc71892b7fec09593f81b1a57ab725c0
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <>
Gerrit-Reviewer: Joe McDonnell <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Tim Armstrong <>

View raw message