[kudu-CR] Kudu Jepsen Tests - Initial Commit
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5492 to look at the new patch set (#7). Change subject: Kudu Jepsen Tests - Initial Commit .. Kudu Jepsen Tests - Initial Commit This patch contains the basic jepsen tests code as it was before KUDU-798 was resolved (i.e. as it was when it was failing). The clojure code is integrated into the Kudu maven build and is compiled along with the other projects. Tests are currently skipped as they require tests infrastructure. Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea --- A java/kudu-jepsen/.gitignore A java/kudu-jepsen/pom.xml A java/kudu-jepsen/resources/kudu.flags A java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj M java/pom.xml 10 files changed, 524 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/7 -- To view, visit http://gerrit.cloudera.org:8080/5492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Kudu Jepsen Tests - Initial Commit
David Ribeiro Alves has posted comments on this change. Change subject: Kudu Jepsen Tests - Initial Commit .. Patch Set 6: (11 comments) http://gerrit.cloudera.org:8080/#/c/5492/6//COMMIT_MSG Commit Message: PS6, Line 17: WIP as this still needs some cleanup/minimal docs but : should be reasonably ready for a first review iteration. > Should probably be updated now that this is going to be committed, right? Done http://gerrit.cloudera.org:8080/#/c/5492/6/java/kudu-jepsen/.gitignore File java/kudu-jepsen/.gitignore: Line 1: /store > What is this for? Perhaps add a comment just before it? Done http://gerrit.cloudera.org:8080/#/c/5492/6/java/kudu-jepsen/README.md File java/kudu-jepsen/README.md: > Let's either rewrite this to be useful or remove it altogether. removed this, for now http://gerrit.cloudera.org:8080/#/c/5492/6/java/kudu-jepsen/pom.xml File java/kudu-jepsen/pom.xml: PS6, Line 26: : true : > Why this? Add an XML comment to explain. Done Line 31: > Remove this. Done Line 49: 0.1.3 > I think our convention is to list all of our dependent versions in the main that is not true. see kudu-spark, kudu-flume-sink etc. We do, however declare the versions on a section. Done that. Line 56: > Why the slf4j-api exclusion here and below? Add a comment explaining. Done Line 89: true > If this is a default value, can you remove it? If not, could you add a comm its disabled by default, added a comment 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 17: ;; TODO allow to replace the binaries with locally built ones > Yes. I assume Maven is used to launch the Jepsen tests? If so, it would be This is an initial version of the jepsen test runs that only runs against the nightlies. I'd rather add that when we support running against different kudu versions. We should decide then what is the best way to define kudu repo locations. Line 74: (c/su > So the obvious question is: why do we need to use system packages with Jeps For expediency, mostly. However I don't think there is much to gain from running jepsen tests against different platforms. I'd rather have a first version that runs against already-provisioned debian/ubuntu nodes and consider whether it's worth it to add support for different platforms afterwards (which, at least for now, I can't think of a reason why it'd be worthwhile) http://gerrit.cloudera.org:8080/#/c/5492/6/java/pom.xml File java/pom.xml: Line 308:
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5492 to look at the new patch set (#8). Change subject: Kudu Jepsen Tests - Initial Commit .. Kudu Jepsen Tests - Initial Commit This patch contains the basic jepsen tests code as it was before KUDU-798 was resolved (i.e. as it was when it was failing). The clojure code is integrated into the Kudu maven build and is compiled along with the other projects. Tests are currently skipped as they require tests infrastructure. Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea --- M build-support/jenkins/build-and-test.sh A build-support/jenkins/toolchains.xml A java/kudu-jepsen/.gitignore A java/kudu-jepsen/pom.xml A java/kudu-jepsen/resources/kudu.flags A java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj M java/pom.xml 12 files changed, 559 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/8 -- To view, visit http://gerrit.cloudera.org:8080/5492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5492 to look at the new patch set (#9). Change subject: Kudu Jepsen Tests - Initial Commit .. Kudu Jepsen Tests - Initial Commit This patch contains the basic jepsen tests code as it was before KUDU-798 was resolved (i.e. as it was when it was failing). The clojure code is integrated into the Kudu maven build and is compiled along with the other projects. Tests are currently skipped as they require tests infrastructure. Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea --- M build-support/jenkins/build-and-test.sh A build-support/jenkins/toolchains.xml A java/kudu-jepsen/.gitignore A java/kudu-jepsen/pom.xml A java/kudu-jepsen/resources/kudu.flags A java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj A java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj M java/pom.xml 12 files changed, 560 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/9 -- To view, visit http://gerrit.cloudera.org:8080/5492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Kudu Jepsen Tests - Initial Commit
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 Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [jenkins] added JEPSEN build configuration
Alexey Serbin has posted comments on this change. Change subject: [jenkins] added JEPSEN build configuration .. Patch Set 3: > As Todd mentioned, the upstream Kudu Jenkins instance probably > can't communicate with internal Cloudera resources, so even if > build-and-test.sh was configured with the new parameters correctly, > it likely won't work. Plus, it's probably overkill to run Jepsen > precommit anyway. > > I think you've pretty much abandoned this in favor of a dedicated > Cloudera-internal jepsen nightly test, right? This script is used to run http://sandbox.jenkins.cloudera.com/view/Kudu/job/kudu-jepsen/ I can separate this into another file, if needed. -- To view, visit http://gerrit.cloudera.org:8080/5559 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia2576717dc8fff3fe2c275f29c2c06eb0d35bddc Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Alexey Serbin has posted comments on this change. Change subject: Kudu Jepsen Tests - Initial Commit .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/5492/6/java/pom.xml File java/pom.xml: Line 308:
[kudu-CR] WIP [jepsen.kudu] run tests from clojure-maven-plugin
Alexey Serbin has posted comments on this change. Change subject: WIP [jepsen.kudu] run tests from clojure-maven-plugin .. Patch Set 7: (1 comment) Thank you for the review! I'll address the comments a little bit later. http://gerrit.cloudera.org:8080/#/c/5551/7/java/kudu-jepsen/README.md File java/kudu-jepsen/README.md: Line 16: Note that commas (not spaces) are used to separate the names of the machines. > Would it be possible to let the shell expand the node list? For example: I'm not sure that's possible: the issue here is with passing those parameters into maven plugin if those parameters are separated by space (even without any expansion). In other words, having -DtserverNodes="a b c" in command line does not work. Of course, it would be nice to fix that, but I'm not sure that's the thing we want to spend our time. However, if the workaround exists, we could try to use it. -- To view, visit http://gerrit.cloudera.org:8080/5551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5adb6968df46954f94c11f29ecc4dd4ea56544b1 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] [jenkins] added JEPSEN build configuration
Adar Dembo has posted comments on this change. Change subject: [jenkins] added JEPSEN build configuration .. Patch Set 3: > > I think you've pretty much abandoned this in favor of a dedicated > > Cloudera-internal jepsen nightly test, right? > > This script is used to run > http://sandbox.jenkins.cloudera.com/view/Kudu/job/kudu-jepsen/ > > I can separate this into another file, if needed. Oh I see. Do you think this approach is cleaner than a standalone script a la benchmarks.sh or tpch.sh? -- To view, visit http://gerrit.cloudera.org:8080/5559 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia2576717dc8fff3fe2c275f29c2c06eb0d35bddc Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Alexey Serbin has posted comments on this change. Change subject: Kudu Jepsen Tests - Initial Commit .. Patch Set 6: (2 comments) Thank you for the review! Certainly we should communicate on our expectations and requirements. I considered this original David's patch as a foundation to build the necessary functionality to run jepsen test against freshly built Kudu binaries. You can find the result at: http://sandbox.jenkins.cloudera.com/view/Kudu/job/kudu-jepsen/ I decided to address some comments right away to resolve the ambiguity and suspense. I'm planning to get back on the rest when I'm back from vacation. I'm not sure what would be the best way to proceed here: merge everything into one patch or something else. As you can see, I have two additional patches based on this: https://gerrit.cloudera.org/#/c/5500/ https://gerrit.cloudera.org/#/c/5551/ https://gerrit.cloudera.org/#/c/5559/ I hope that answers at least some questions. 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 > So the obvious question is: why do we need to use system packages with Jeps In my subsequent patch there are provisions to run that with binaries only. That was the original approach that David and I were running at our laptops in Docker for Mac. I think we need two options here: (a) run against built packages (Debian only due to Jepsen restrictions) and (b) run against freshly built binaries. In http://sandbox.jenkins.cloudera.com/view/Kudu/job/kudu-jepsen/ the latter approach is used. And that will be an option. Line 74: (c/su > In general, it's much easier to run tests without root access on the remote The root access is needed to manage IP filter and other nemesis (i.e. failure-injection) stuff -- that's how Jepsen does its failure injection thing on DB nodes. I don't think we are about to re-do that part, at least in the nearest future. Ideally, it would be nice to run everything using our minicluster (as Dan already mentioned somewhere), but that would result in invasive changes on Jepsen core code. So, I would opt for keeping requirement for root access at remote nodes, so we are conformant with Jepsen infrastructure. The NTP part could be removed if we add a separate provision in our Kudu source to ignore non-synced time -- that we will need eventually anyway (I started that patch, but it's not ready for review yet). But as for now I don't see a problem here. -- 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 Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [jenkins] added JEPSEN build configuration
Alexey Serbin has posted comments on this change. Change subject: [jenkins] added JEPSEN build configuration .. Patch Set 3: > > > I think you've pretty much abandoned this in favor of a > dedicated > > > Cloudera-internal jepsen nightly test, right? > > > > This script is used to run > > http://sandbox.jenkins.cloudera.com/view/Kudu/job/kudu-jepsen/ > > > > I can separate this into another file, if needed. > > Oh I see. Do you think this approach is cleaner than a standalone > script a la benchmarks.sh or tpch.sh? I don't think it's cleaner. I just used it because I initially started it as an update for the jenkins build script. By the time I submitted this patch, I still had some wild ideas that with additional provisions to create virtual machines from the script (like those we use for dist_test), we could make it run as a pre-commit check (that was the 'ideal' target we initially discussed with David). Now I understand that the consensus is that this test is more like a system test which should not be run with every commit. That's said, I think the approach like benchmarks.sh or tpch.sh is a better/cleaner fit here. I'll update this patch and the Jenkins job at http://sandbox.jenkins.cloudera.com/view/Kudu/job/kudu-jepsen/ accordingly. -- To view, visit http://gerrit.cloudera.org:8080/5559 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia2576717dc8fff3fe2c275f29c2c06eb0d35bddc Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1817. Fix kudu-spark2 artifactId in pom.xml
Jun He has uploaded a new change for review. http://gerrit.cloudera.org:8080/5586 Change subject: KUDU-1817. Fix kudu-spark2 artifactId in pom.xml .. KUDU-1817. Fix kudu-spark2 artifactId in pom.xml kudu-spark2 pom.xml is malformed due to a maven shade plugin issue: dependency-reduced-pom should use shadedArtifactId (https://issues.apache.org/jira/browse/MSHADE-155) I upgraded maven-shade-plugin to version 2.4 and fixed the pom.xml. Additionally, I moved the default properties to a default profile. This makes the property settings consistent for both versions. Change-Id: I0dc9ac261a82e13d189bc1be04faea5a9812d518 --- M java/kudu-spark/pom.xml 1 file changed, 15 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/5586/1 -- To view, visit http://gerrit.cloudera.org:8080/5586 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0dc9ac261a82e13d189bc1be04faea5a9812d518 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jun He
