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 <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
