Sailesh Mukil has posted comments on this change. Change subject: IMPALA-5750: Catch exceptions from boost thread creation ......................................................................
Patch Set 8: (8 comments) http://gerrit.cloudera.org:8080/#/c/7730/8/be/src/catalog/catalog-server.h File be/src/catalog/catalog-server.h: PS8, Line 39: class TGetAllCatalogObjectsResponse; nit: Not your change, but no longer required. http://gerrit.cloudera.org:8080/#/c/7730/7/be/src/exec/kudu-scan-node.cc File be/src/exec/kudu-scan-node.cc: PS7, Line 173: scanner_threads_.AddThread(move(t)); > Here is my understanding: Good point, I was looking at the base code for Close() instead of your updated version where you added SetDone(). Looks like you may have fixed this previously existing subtle race by adding that. PS7, Line 240: SetDoneInternal(); We previously didn't call materialized_row_batches_->Shutdown() here, but now we will, is that acceptable? http://gerrit.cloudera.org:8080/#/c/7730/8/be/src/rpc/auth-provider.h File be/src/rpc/auth-provider.h: PS8, Line 22: #include <boost/scoped_ptr.hpp> No longer needed. http://gerrit.cloudera.org:8080/#/c/7730/8/be/src/runtime/disk-io-mgr-handle-cache.h File be/src/runtime/disk-io-mgr-handle-cache.h: Line 28: #include "common/hdfs.h" nit: Include what you use, status.h (it's already being pulled in through one of the other headers anyway) http://gerrit.cloudera.org:8080/#/c/7730/8/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: PS8, Line 341: admission_controller_->Init() The admission controller dequeue thread starts a little later than before now. I just want to confirm if that's okay. http://gerrit.cloudera.org:8080/#/c/7730/8/be/src/service/child-query.h File be/src/service/child-query.h: PS8, Line 22: <boost/thread.hpp> boost/thread/mutex.hpp http://gerrit.cloudera.org:8080/#/c/7730/8/be/src/statestore/statestore.h File be/src/statestore/statestore.h: Line 30: nit: include what you use: status.h -- 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
