impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5750: Catch exceptions from boost thread creation
Date Fri, 01 Sep 2017 19:44:49 GMT
Dan Hecht has posted comments on this change.

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


Patch Set 8:

(2 comments)

I need to go through query-state one more time, but otherwise here's my remaining comments.

http://gerrit.cloudera.org:8080/#/c/7730/8/be/src/rpc/thrift-thread.cc
File be/src/rpc/thrift-thread.cc:

PS8, Line 40: throw atc::SystemResourceException("Thread::Create() failed");
is it worth incorporating the status msg into the exception message?


http://gerrit.cloudera.org:8080/#/c/7730/8/be/src/runtime/thread-resource-mgr.h
File be/src/runtime/thread-resource-mgr.h:

Line 288:   if (!in_callback) InvokeCallbacks();
This explains why we need to skip the callbacks. But why is it safe to do so? The callbacks
are invoked here to give another thread a chance to start up, so why is it safe to skip that
just because we're inside the callback? (Given the current callsite, it is safe, but it doesn't
seem intuitively true when inside a callback generally).


-- 
To view, visit http://gerrit.cloudera.org:8080/7730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15a2f278dc71892b7fec09593f81b1a57ab725c0
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message