Michael Brown has posted comments on this change. Change subject: IMPALA-3200: more buffer pool end-to-end tests ......................................................................
Patch Set 5: (7 comments) General comment: For some of the end-to-end tests, you've used workloads that will never be run in exhaustive [0]. I've marked such tests below. If someone decides later to skip these tests except when running in exhaustive, the net effect will be that the tests are always skipped. Absent a wholesale fix to IMPALA-3947, do you think in the mean time it makes sense to add a note saying that the tests must run in core, not exhaustive? [0] See the following for more info: - IMPALA-3947 - IMPALA-3630 and associated code review - https://gerrit.cloudera.org/#/c/6002/ http://gerrit.cloudera.org:8080/#/c/7552/5/testdata/workloads/functional-query/queries/QueryTest/spilling-sorts-exhaustive.test File testdata/workloads/functional-query/queries/QueryTest/spilling-sorts-exhaustive.test: PS5, Line 86: from (select cast(l_comment as char(200)) char_col I realize this is a move, but is a varchar(200) test similar to this one needed? http://gerrit.cloudera.org:8080/#/c/7552/5/testdata/workloads/targeted-perf/targeted-perf_dimensions.csv File testdata/workloads/targeted-perf/targeted-perf_dimensions.csv: PS5, Line 1: file_format: text,seq,rc,avro,parquet,kudu Thanks for adding this extra coverage here and elsewhere. Have you verified in a Jenkins job that the extra coverage directives are working and that the correct new tests are being run? http://gerrit.cloudera.org:8080/#/c/7552/5/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: PS5, Line 321: impalad_args=impalad_admission_ctrl_flags(1, 1, 10 * 1024 * 1024, : 1024 * 1024 * 1024) Nit: There are so many numbers here it might be nice to consider kwargs-style arguments in the future so a reader understands what maps to what. PS5, Line 332: assert re.search("Failed to get minimum memory reservation", str(ex)) Micro-optimization nit: You can use the in operator in python for substring matching since you're not using any regex special patterns. http://gerrit.cloudera.org:8080/#/c/7552/5/tests/query_test/test_mem_usage_scaling.py File tests/query_test/test_mem_usage_scaling.py: PS5, Line 92: return 'tpch' Workload will not run under exhaustive. PS5, Line 108: print str(result) Remove? PS5, Line 254: return 'targeted-perf' Workload will not run under exhaustive. -- To view, visit http://gerrit.cloudera.org:8080/7552 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I554aa5ddfef4f8e75295596e720a14eee1afa17f Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
