impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Knupp (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4259: build Impala without any test cluster setup.
Date Wed, 12 Oct 2016 00:38:26 GMT
David Knupp has posted comments on this change.

Change subject: IMPALA-4259: build Impala without any test cluster setup.

Patch Set 1:


PS1, Line 386: 5
> No idea, I just moved the preexisting code. 
If a more meaningful test could be substituted, that would be great. If not, then I think
I agree -- removing it would probably be better.

PS1, Line 424: start_test_cluster
> It is a little weird. I guess because the test or data loading scripts want
I don't want to hold up this review, since this is a clear improvement on what exists, so
consider this speculative. It would be nice if the outcome of the script were
simply a running Impala cluster, whether or not data gets loaded or tests get run -- e.g.,
perhaps by adding a line to call with no params if neither TESTDATA_ACTION
nor TESTS_ACTION are true.

In fact, you could uncouple that line from the "if [[ ${IMPALA_KERBERIZE} -eq 1 ]]" block,
and add it in after the fact. In other words:

  if [[ ${TESTDATA_ACTION} -eq 0  &&  ${TESTS_ACTION} -eq 0 ]]; then

(My bash still kinda sucks, so that might not be the exact right syntax.)

This way, we won't wind up with this one oddball condition whereby if we run non-kerberized,
but without loading up data or running tests, we don't even bother starting up Impala. Otherwise,
the starting of Impala will be handled by the other scripts. 

(FWIW, from a cursory glance at those other scripts, it appears that they each employ some
basic logic to determine the startup params to use for Impala (e.g., log_dir location, cluster_size,
impalad_args, etc. etc.), and then independently call So it actually
gets called more than once if you are both loading data and then also running tests.)

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I429da7bc6681b16c07fe58bb3efac6d1a8579137
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: David Knupp <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message