impala-reviews mailing list archives

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

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

Patch Set 8:

File be/src/catalog/catalog-server.h:

PS8, Line 39: class TGetAllCatalogObjectsResponse;
nit: Not your change, but no longer required.
File be/src/exec/

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?
File be/src/rpc/auth-provider.h:

PS8, Line 22: #include <boost/scoped_ptr.hpp>
No longer needed.
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)
File be/src/runtime/

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.
File be/src/service/child-query.h:

PS8, Line 22: <boost/thread.hpp>
File be/src/statestore/statestore.h:

Line 30: 
nit: include what you use: status.h

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I15a2f278dc71892b7fec09593f81b1a57ab725c0
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Joe McDonnell <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message