impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Harrison Sheinblatt (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] Enabling end-to-end tests on a remote cluster
Date Wed, 26 Oct 2016 01:02:37 GMT
Harrison Sheinblatt has posted comments on this change.

Change subject: Enabling end-to-end tests on a remote cluster
......................................................................


Patch Set 1:

(3 comments)

Responded to comments.

http://gerrit.cloudera.org:8080/#/c/4769/1/testdata/bin/compute-table-stats.sh
File testdata/bin/compute-table-stats.sh:

PS1, Line 27: IMPALAD
> IMPALA-4346 has been filed.
Can you reference the Jira in a comment?


http://gerrit.cloudera.org:8080/#/c/4769/1/testdata/bin/create-load-data.sh
File testdata/bin/create-load-data.sh:

PS1, Line 38: HS2_HOST_PORT
> I think the latter is preferable -- corral all the required configs in one 
Is it reasonable to add a comment referencing the Jira here?


http://gerrit.cloudera.org:8080/#/c/4769/1/testdata/bin/setup-hdfs-env.sh
File testdata/bin/setup-hdfs-env.sh:

PS1, Line 53: CACHEADMIN_ARGS
> Clarification: can you be more explicit about the check you want? Something
If the is_kerberized block is executed above, then the CACHADMIN_ARGS would include '-owner
${PREVIOUS_USER}'.  If HADOOP_USER_NAME is also true, then we add another '-owner ${USER}'
to this, which probably breaks it.  I think there are probably 4 bugs: 1) The is_kerberized
block above probably isn't supported and should be removed and 2) the CACHEADMIN_ARGS definition
logic needs a clear conditional, ideally in a single location, that sets the user/group/owner
information properly in a way that you can easily tell it's always well-defined.  Here it
looks like the logic is intended to be that if it's kerberized it sets owner one way, if it's
not kerberized and the hadoop user is defined it's set another way and if it's not kerberized
and the hadoop user is not defined it stays undefined. If we want to keep the is_kerberized
logic in one place, then we can have it set another parameter about owner fields and here
only update it if it's set already.  3) CACHEADMIN_ARGS is prob!
 ably the wrong name as it is for the -addPool command and sets a subset of the args 4) We
should explicitly set all arguments to cacheadmin, -addPool if possible (e.g. mode maxTtl)


-- 
To view, visit http://gerrit.cloudera.org:8080/4769
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1f443a1728a1d28168090c6f54e82dec2cb073e9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Harrison Sheinblatt <hs7@hotmail.com>
Gerrit-Reviewer: Martin Grund <grundprinzip@gmail.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message