Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4996: Single-threaded KuduScanNode ......................................................................
Patch Set 2: (5 comments) I started reviewing this and realized there are maybe some more changes you're still working on to share the KuduClient per-query, so I'll hold off on looking much more closely. Looks good so far though. http://gerrit.cloudera.org:8080/#/c/6312/2//COMMIT_MSG Commit Message: Line 1: Parent: def5355a (IMPALA-3403: [DOCS] Pare back irrelevant installation info) nit: can you wrap lines in git commits at 60 chars? formatting in git tools prefer this If you edit commit messages with git you can have it auto set this w/ this in your vimrc: au FileType gitcommit set tw=60 http://gerrit.cloudera.org:8080/#/c/6312/2/be/src/exec/kudu-scan-node-base.cc File be/src/exec/kudu-scan-node-base.cc: PS2, Line 122: Base remove Base for the debugging output http://gerrit.cloudera.org:8080/#/c/6312/2/be/src/exec/kudu-scan-node.h File be/src/exec/kudu-scan-node.h: PS2, Line 79: : RuntimeProfile::Counter* kudu_round_trips_; : RuntimeProfile::Counter* kudu_remote_tokens_; : static const std::string KUDU_ROUND_TRIPS; : static const std::string KUDU_REMOTE_TOKENS; remove; aren't these in the base class? http://gerrit.cloudera.org:8080/#/c/6312/2/tests/query_test/test_mt_dop.py File tests/query_test/test_mt_dop.py: PS2, Line 102: rlies typo PS2, Line 118: # Parquet filtering test rlies on a specific mt_dop value, so keep in its own test update comment -- To view, visit http://gerrit.cloudera.org:8080/6312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6e4593300e376bc508b78acaea64ffdd2c73a67a Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
