Adar Dembo has posted comments on this change.

Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/5492/9/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

PS9, Line 90: 
            : # Set up default
How about just "echo JAVA8_HOME: $JAVA8_HOME"? That way it's more clear if it's 
unset, plus it's also clear from the output that you're testing the env 
variable with that exact name.

Though, I should ask, what exactly is this for?


http://gerrit.cloudera.org:8080/#/c/5492/9/build-support/jenkins/toolchains.xml
File build-support/jenkins/toolchains.xml:

Line 4
OK, but effect does this file have? Would be nice to explain in this comment, 
or somewhere else.


Line 11
This looks like a macOS path. What's it doing in a jenkins/... file?


Line 13
Nit: got some extra whitespace here.


http://gerrit.cloudera.org:8080/#/c/5492/6/java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj:

Line 74:       (c/su
> For expediency, mostly. However I don't think there is much to gain from ru
In general, it's much easier to run tests without root access on the remote 
nodes than with. I didn't emphasize that in my comment, but that's really what 
I'm driving at here: can we avoid using root?

Part of that is not using system packages, which obviously require root to be 
installed.

Part of that is also the ntp configuration; is it highly customized, or what 
you'd find in a typical deployment? (I asked Alexey about this on HipChat; I 
forgot you were going to take over this patch again).

I don't know how Jepsen does what it does; maybe this is just the tip of the 
iceberg and root is needed all over the place (if so, I would have liked to see 
that (important) requirement documented in the commit message). But my goal is 
to have a conversation about what's possible and what's not, and to learn about 
Jepsen's overall system requirements, which haven't been  well communicated yet.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to