Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16385 )
Change subject: IMPALA-9792: Implement splitting kudu scan ranges for greater parallelism ...................................................................... Patch Set 1: (4 comments) This seems I think basically OK. I'm on the fence about whether we should do some additional cluster testing, but leaning towards no because the complexity is all in the Kudu layer and I don't think we'd learn much from testing at small scale. http://gerrit.cloudera.org:8080/#/c/16385/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16385/1//COMMIT_MSG@24 PS1, Line 24: Testing > Did you do any performance testing to gauge the impact (good or bad)? Yeah it'd be good to do some sanity checks to confirm that it gives the speedup expected. E.g. TPC-H or similar. Maybe just a single query would be fine, e.g. TPC-H Q1. http://gerrit.cloudera.org:8080/#/c/16385/1//COMMIT_MSG@25 PS1, Line 25: - Added e2e tests Do we have other any other tests that are going to exercise this code path, e.g. kudu e2e tests with multithreading. http://gerrit.cloudera.org:8080/#/c/16385/1/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/16385/1/tests/query_test/test_kudu.py@1442 PS1, Line 1442: union union all would be a little more efficient, no? http://gerrit.cloudera.org:8080/#/c/16385/1/tests/query_test/test_kudu.py@1468 PS1, Line 1468: assert regular_num_inst < with_mt_dop_and_disabled_range_len_num_inst < \ : with_mt_dop_num_inst < with_mt_dop_and_low_range_len_num_inst I don't think the < operator works this way - you're going to be comparing a bool with an in. Probably best to have each inequality as a separate assert. -- To view, visit http://gerrit.cloudera.org:8080/16385 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia02fd94cc1d13c61bc6cb0765dd2cbe90e9a5ce8 Gerrit-Change-Number: 16385 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Mon, 31 Aug 2020 23:44:46 +0000 Gerrit-HasComments: Yes
