Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10186 )
Change subject: IMPALA-6920: fix inconsistencies with scanner thread tokens ...................................................................... IMPALA-6920: fix inconsistencies with scanner thread tokens The first scanner thread to start now takes a "required" token, which always succeeds. Only additional threads try to get "optional" tokens, which can fail. Previously threads always requested optional tokens, which could fail and leave the scan node without any running threads until its callback is invoked. This allows us to remove the "reserved optional token" and set_max_quota() interfaces from ThreadResourceManager. There should be no behavioural changes in ThreadResourceMgr in cases when those features are not used. Also switch Kudu to using the same logic for implementing NUM_SCANNER_THREADS (it was not switched over to the improved HDFS scanner logic added in IMPALA-2831). Do some cleanup in ThreadResourceMgr code while we're here: * Fix some benign data races in ThreadResourceMgr by switching to AtomicInt* classes. * Remove pointless object caching (TCMalloc will do better). * Reduce dependencies on the thread-resource-mgr.h header. Testing: Ran core tests. Ran a few queries under TSAN, checked that it didn't report any more races in this code after fixing those data races. I couldn't construct a regression test because there are no easily testable consequences of the change - the main difference is that some scanner threads start earlier when there is pressure on scanner thread tokens but that is hard to construct a robust test around. Change-Id: I16d31d72441aff7293759281d0248e641df43704 Reviewed-on: http://gerrit.cloudera.org:8080/10186 Reviewed-by: Tim Armstrong <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- M be/src/exec/blocking-join-node.cc M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scan-node.h M be/src/exec/kudu-scan-node.cc M be/src/exec/kudu-scan-node.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/io/disk-io-mgr-internal.h M be/src/runtime/io/disk-io-mgr.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/thread-resource-mgr-test.cc M be/src/runtime/thread-resource-mgr.cc M be/src/runtime/thread-resource-mgr.h 13 files changed, 290 insertions(+), 337 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10186 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I16d31d72441aff7293759281d0248e641df43704 Gerrit-Change-Number: 10186 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]>
