impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5167: Reduce the number of Kudu clients created
Date Tue, 09 May 2017 18:28:12 GMT
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5167: Reduce the number of Kudu clients created

Patch Set 2:

Commit Message:

Line 22: 
> Turns out we already have a test that covers this -
did your manual testing involve concurrency? I think it's still an area for tests
Commit Message:

PS2, Line 20: fdc022fe6231af20e307012d98c35b16cbfa7b33
While this change doesn't have a compile time dependency on the client, this does need these
changes to come into our C++ client via the toolchain.

(...and I'm having issues with my toolchain update.)
File be/src/exec/

Line 80:       client_(nullptr),
move to the header

PS2, Line 83:       total_rows_(NULL),
            :       num_row_errors_(NULL),
            :       rows_processed_rate_(NULL)
can you set these to nullptr as well in the header

replace all NULLs will nullptrs in file

Line 145:   Status s =
fit on 1line?
File be/src/exec/kudu-table-sink.h:

PS2, Line 103: client_
set to nullptr here
File be/src/runtime/

PS2, Line 403: master_addresses
can you sort the list before joining? That way we can avoid dupes between serverA,serverB
 and serverB,serverA
File be/src/runtime/exec-env.h:

PS2, Line 193: fragment instance
this is wrong now

PS2, Line 193: Each Kudu table has
             :   /// a list of master addresses stored in the Hive Metastore. 
I don't think this sentence is relevant. Remove

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b0c12a256c33e8ef32315b3736cae2dea2ae705
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Thomas Tauber-Marshall <>
Gerrit-HasComments: Yes

View raw message