You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2016/12/13 21:52:54 UTC
[kudu-CR] WIP: Kudu Jepsen Tests - Initial Commit
David Ribeiro Alves has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5492
Change subject: WIP: Kudu Jepsen Tests - Initial Commit
......................................................................
WIP: 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.
WIP as this still needs some cleanup/minimal docs but
should be reasonably ready for a first review iteration.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/CHANGELOG.md
A java/kudu-jepsen/LICENSE
A java/kudu-jepsen/README.md
A java/kudu-jepsen/doc/intro.md
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/project.clj
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
15 files changed, 784 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/1
--
To view, visit http://gerrit.cloudera.org:8080/5492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 6:
(11 comments)
I only skimmed the Jepsen code; mostly I was looking at how this integrates into the rest of the system.
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?
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?
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.
http://gerrit.cloudera.org:8080/#/c/5492/6/java/kudu-jepsen/pom.xml
File java/kudu-jepsen/pom.xml:
PS6, Line 26: <properties>
: <skipTests>true</skipTests>
: </properties>
Why this? Add an XML comment to explain.
Line 31:
Remove this.
Line 49: <version>0.1.3</version>
I think our convention is to list all of our dependent versions in the main pom.xml rather than in each subproject.
Line 56: <exclusions>
Why the slf4j-api exclusion here and below? Add a comment explaining.
Line 89: <extensions>true</extensions>
If this is a default value, can you remove it? If not, could you add a comment explaining why we're using it?
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 nice if these were specified as maven properties that could be overridden.
Line 74: (c/su
So the obvious question is: why do we need to use system packages with Jepsen? If we could use regular old binaries:
1) We could run Jepsen on el platforms too, and
2) I don't think we'd need to be root.
http://gerrit.cloudera.org:8080/#/c/5492/6/java/pom.xml
File java/pom.xml:
Line 308: <!-- Build the jepsen test for Kudu.
Why hide the jepsen tests behind a disabled-by-default Maven profile? Is it impossible to build without some custom system configuration?
--
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 <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP: Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: WIP: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/5492/4/java/kudu-jepsen/project.clj
File java/kudu-jepsen/project.clj:
Line 1: (defproject kudu "0.1.0-SNAPSHOT"
> Is this file necessary? Seems like we should have either the pom.xml or pr
It seems you are right -- we don't need this one since we have pom.xml which uses maven-clojure-plugin. Removing.
http://gerrit.cloudera.org:8080/#/c/5492/4/java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj:
Line 32: (when (> (count (:masters test)) 1)
> Why only set the master addresses when there are more than one master? Thi
Good idea -- I'll do this in one of the follow-up patches.
PS4, Line 54: \u2028
> trailing whitespace
Done
PS4, Line 59: into []
> I don't think the 'into []' is necessary, str/join should work fine over a
Done
PS4, Line 59: efn
> this could be a def
I would prefer to keep some symmetry here: ntp-server-config and ntp-slave-config are both functions.
PS4, Line 63: into []
> Same.
Done
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#6).
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.
WIP as this still needs some cleanup/minimal docs but
should be reasonably ready for a first review iteration.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.md
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
11 files changed, 535 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/6
--
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: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
Patch Set 22:
I'm merging this anyway. We can address any docker specific issues in follow up patches.
--
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 12:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5492/12/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:
Line 90: echo "Outputting java 8 home"
I agree with Adar's comment on rev 9 about removing or consolidating this to 1 line.
PS12, Line 358: /opt/apache-maven-3.3.9/bin
> Are we ready to migrate to the new maven to build everything Java?
I tend to agree - just because we use JDK 8 to compile kudu-client and the rest doesn't mean they lose compatibility with JRE 7. The -target flag to javac & maven can ensure compat.
Even moreso with bumping maven - as far as I know there aren't any hazards to packaging with maven 3.3 which would make the jar incompatible with maven 3.2 or below.
--
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#16).
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was failing)
and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other unsorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH. The private SSH key should be set prior
to running the test:
1. The public part of the SSH key should be added into the
'authorized_keys' file for the root user on all cluster nodes.
2. The private part of the SSH key should be provided to the test
either by:
* adding the key into the SSH agent on the control node
* specifying the path to the key via 'sshKeyPath' property
Having the Kudu cluster provisioned and SSH keys deployed, to run
the tests against the cluster with master node m0 and tserver nodes
{t0..t4}, build the Kudu project from sources and then execute
the following in the $KUDU_HOME/java/kudu-jepsen directory:
mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
after bulding the top-level project with
mvn clean compile test-compile -Pjepsen
Restrictions:
1. Kudu nodes should run recent Debian/Ubuntu Linux distro
(that's due to the internal Jepsen's restrictions).
2. The 'kudu-master', 'kudu-tserver' and 'kudu' binaries in
the $KUDU_HOME/build/latest/bin should be built for the OS/distro
running on the Kudu cluster nodes.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/resources/ntp.conf.common
A java/kudu-jepsen/resources/ntp.conf.server
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
15 files changed, 1,468 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/16
--
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: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 (#11).
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, 561 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/11
--
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: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5492/4/java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj:
Line 32: (when (> (count (:masters test)) 1)
> Why only set the master addresses when there are more than one master? Thi
As it turned out, there is a check in master_options.cc, in constructor MasterOptions::MasterOptions(), around line 45 -- at least 2 masters are required for a distributed config. Specifying --master_addresses flag means using distributed/clustered master configuration.
I'll update the comment.
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#20).
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was failing)
and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other unsorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH. The private SSH key should be set prior
to running the test:
1. The public part of the SSH key should be added into the
'authorized_keys' file for the root user on all cluster nodes.
2. The private part of the SSH key should be provided to the test
either by:
* adding the key into the SSH agent on the control node
* specifying the path to the key via 'sshKeyPath' property
Having the Kudu cluster provisioned and SSH keys deployed, to run
the tests against the cluster with master node m0 and tserver nodes
{t0..t4}, build the Kudu project from sources and then execute
the following in the $KUDU_HOME/java/kudu-jepsen directory:
mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
after bulding the top-level project with
mvn clean compile test-compile -Pjepsen
Restrictions:
1. Kudu nodes should run recent Debian/Ubuntu Linux distro
(that's due to the internal Jepsen's restrictions).
2. The 'kudu-master', 'kudu-tserver' and 'kudu' binaries in
the $KUDU_HOME/build/latest/bin should be built for the OS/distro
running on the Kudu cluster nodes.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/resources/ntp.conf.common
A java/kudu-jepsen/resources/ntp.conf.server
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
15 files changed, 1,555 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/20
--
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: 20
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP: Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#5).
Change subject: WIP: Kudu Jepsen Tests - Initial Commit
......................................................................
WIP: 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.
WIP as this still needs some cleanup/minimal docs but
should be reasonably ready for a first review iteration.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/CHANGELOG.md
A java/kudu-jepsen/LICENSE
A java/kudu-jepsen/README.md
A java/kudu-jepsen/doc/intro.md
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/project.clj
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
15 files changed, 784 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/5
--
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: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#21).
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was failing)
and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other unsorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH. The private SSH key should be set prior
to running the test:
1. The public part of the SSH key should be added into the
'authorized_keys' file for the root user on all cluster nodes.
2. The private part of the SSH key should be provided to the test
either by:
* adding the key into the SSH agent on the control node
* specifying the path to the key via 'sshKeyPath' property
Having the Kudu cluster provisioned and SSH keys deployed, to run
the tests against the cluster with master node m0 and tserver nodes
{t0..t4}, build the Kudu project from sources and then execute
the following in the $KUDU_HOME/java/kudu-jepsen directory:
mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
after bulding the top-level project with
mvn clean compile test-compile -Pjepsen
Restrictions:
1. Kudu nodes should run recent Debian/Ubuntu Linux distro
(that's due to the internal Jepsen's restrictions).
2. The 'kudu-master', 'kudu-tserver' and 'kudu' binaries in
the $KUDU_HOME/build/latest/bin should be built for the OS/distro
running on the Kudu cluster nodes.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/resources/ntp.conf.common
A java/kudu-jepsen/resources/ntp.conf.server
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
15 files changed, 1,555 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/21
--
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: 21
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#15).
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was failing)
and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other unsorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH. The private SSH key should be set prior
to running the test:
1. The public part of the SSH key should be added into the
'authorized_keys' file for the root user on all cluster nodes.
2. The private part of the SSH key should be provided to the test
either by:
* adding the key into the SSH agent on the control node
* specifying the path to the key via 'sshKeyPath' property
Having the Kudu cluster provisioned and SSH keys deployed, to run
the tests against the cluster with master node m0 and tserver nodes
{t0..t4}, build the Kudu project from sources and then execute
the following in the $KUDU_HOME/java/kudu-jepsen directory:
mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
after bulding the top-level project with
mvn compile -Pjepsen
Restrictions:
1. Kudu nodes should run recent Debian/Ubuntu Linux distro
(that's due to the internal Jepsen's restrictions).
2. The 'kudu-master', 'kudu-tserver' and 'kudu' binaries in
the $KUDU_HOME/build/latest/bin should be built for the OS/distro
running on the Kudu cluster nodes.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/resources/ntp.conf.common
A java/kudu-jepsen/resources/ntp.conf.server
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
15 files changed, 1,386 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/15
--
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: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP: Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: WIP: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 4:
(1 comment)
Thank you for the review! I'll address most of your comments in my follow-up patch, since it would be too cumbersome updating this one and then merge those changes resolving multiple conflicts (my changes are based on top of this change).
Basically, I'd address changes regarding building code with this patch and other things which do not bring too much conflicts.
Would it be OK?
http://gerrit.cloudera.org:8080/#/c/5492/4/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj:
Line 40: (defn kv-table-options
> Any reason not to use hash partitioning? Would make this a lot easier, and
Good point. I don't think there any particular reason for not using hash partitioning. I think we can make it an option and exercise both. But by default it should be hash partitioning, I think.
I'll make that change in the follow-up patch.
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
Patch Set 18:
(7 comments)
final q: does this still work with docker instances?
http://gerrit.cloudera.org:8080/#/c/5492/18//COMMIT_MSG
Commit Message:
PS18, Line 10: failing)
nit: wrap this
PS18, Line 15: unsorted
you mean asorted, no? (i.e. miscellaneous, not out of order)
PS18, Line 25: All Jepsen control operations on the DB nodes (i.e. Kudu master and
: tserver nodes) are run via SSH. The private SSH key should be set prior
: to running the test:
:
: 1. The public part of the SSH key should be added into the
: 'authorized_keys' file for the root user on all cluster nodes.
:
: 2. The private part of the SSH key should be provided to the test
: either by:
: * adding the key into the SSH agent on the control node
: * specifying the path to the key via 'sshKeyPath' property
:
: Having the Kudu cluster provisioned and SSH keys deployed, to run
: the tests against the cluster with master node m0 and tserver nodes
: {t0..t4}, build the Kudu project from sources and then execute
: the following in the $KUDU_HOME/java/kudu-jepsen directory:
:
: mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
:
: after bulding the top-level project with
:
: mvn clean compile test-compile -Pjepsen
don't think this info needs to be here. Is it in the README?
http://gerrit.cloudera.org:8080/#/c/5492/18/java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj:
PS18, Line 87: into
s/into/to
same below
http://gerrit.cloudera.org:8080/#/c/5492/18/java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj:
PS18, Line 99: but no server sees
: the *same* majority as any other.
is there a guarantee that the majorities are different, or is it just likely that they are?
http://gerrit.cloudera.org:8080/#/c/5492/18/java/kudu-jepsen/src/main/clojure/jepsen/kudu/util.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/util.clj:
PS18, Line 34: 1.2.0
isn't this 1.3.0-SNAPSHOT or whatever now?
http://gerrit.cloudera.org:8080/#/c/5492/18/java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
File java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj:
PS18, Line 53: (def register-test-configs
: [
: {:suffix "tserver-random-halves"
: :nemesis '(kn/tserver-partition-random-halves)}
: {:suffix "tserver-majorities-ring"
: :nemesis '(kn/tserver-partition-majorities-ring)}
: {:suffix "kill-restart-2-tservers"
: :nemesis '(kn/kill-restart-tserver (comp (partial take 2) shuffle))}
: {:suffix "kill-restart-3-tservers"
: :nemesis '(kn/kill-restart-tserver (comp (partial take 3) shuffle))}
: {:suffix "kill-restart-all-tservers"
: :nemesis '(kn/kill-restart-tserver shuffle)}
: {:suffix "all-random-halves"
: :nemesis '(jn/partition-random-halves)}
: {:suffix "all-majorities-ring"
: :nemesis '(jn/partition-majorities-ring)}
: {:suffix "hammer-2-tservers"
: :nemesis '(kn/tserver-hammer-time (comp (partial take 2) shuffle))}
: {:suffix "hammer-3-tservers"
: :nemesis '(kn/tserver-hammer-time (comp (partial take 3) shuffle))}
: {:suffix "hammer-all-tservers"
: :nemesis '(kn/tserver-hammer-time shuffle)}
: ])
clap clap! neato, thanks for adding all there scenarios
--
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: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#14).
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was failing)
and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other unsorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH. The private SSH key should be set prior
to running the test:
1. The public part of the SSH key should be added into the
'authorized_keys' file for the root user on all cluster nodes.
2. The private part of the SSH key should be provided to the test
either by:
* adding the key into the SSH agent on the control node
* specifying the path to the key via 'sshKeyPath' property
Having the Kudu cluster provisioned and SSH keys deployed, to run
the tests against the cluster with master node m0 and tserver nodes
{t0..t4}, build the Kudu project from sources and then execute
the following in the $KUDU_HOME/java/kudu-jepsen directory:
mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
after bulding the top-level project with
mvn compile -Pjepsen
Restrictions:
1. The Kudu cluster should consist of Linux machines of the same
architecture and OS distro as the machine where the Kudu
kudu-master, kudu-tserver, and the 'kudu' CLI utility are built.
2. As for now, the kudu-jepsen test runs on recent Debian distros
only (jepsen supports only Debian Linux).
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/resources/ntp.conf.common
A java/kudu-jepsen/resources/ntp.conf.server
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
15 files changed, 1,386 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/14
--
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: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
Patch Set 16:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/.gitignore
File java/kudu-jepsen/.gitignore:
Line 1: # Ignore files containing information on prior Leiningen runs/sessions.
> add apache header
Done
Line 5: /store
> why do you need '/store' instead of just 'store' or 'store/'?
It seems it's a typo -- should be store/ instead.
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/README.adoc
File java/kudu-jepsen/README.adoc:
PS16, Line 57: The Kudu master and tserver nodes should be created prior to running the test:
: the tests does not create those itself
> can you clarify whether you just mean that the machines should be created,
Done
PS16, Line 147: liear
> typo
Done
PS16, Line 152: activisation
> activation
Done
PS16, Line 156: correspondning
> corresponding
Done
PS16, Line 158: should under
> should be under
Done
PS16, Line 162: The Jepsen code is distributed under the Eclipse Public License either
: version 1.0 or (at your option) any later version.
> If we're not shipping Jepsen itself, I dont think it's necessary to specify
Done
Line 165: The kudu-jepsen is licensed under the Apache License, Version 2.0
> this should be implicitly covered by the top-level LICENSE file we already
Done
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/resources/kudu.flags
File java/kudu-jepsen/resources/kudu.flags:
Line 1: # This overrides all flags in a flag file
> nit: add license (or add to rat excludes if not possible)
Done
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/resources/ntp.conf.common
File java/kudu-jepsen/resources/ntp.conf.common:
Line 1: tinker panic 0
> same (license)
Done
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj:
PS16, Line 92: do (let [rr-iter (.nextRows scanner)]
: (while (.hasNext rr-iter)
: (do
> are these two 'do's necessary given the single-expression loop bodies?
Good point: I found it's not necessary here, but some tutorial have (do) in their examples: https://www.tutorialspoint.com/clojure/clojure_while_statement.htm
I'll remove that (do) -- at least it seems working without (do).
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj:
PS16, Line 34: respond
> response
Done
PS16, Line 35: respond
> response
Done
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj:
PS16, Line 71: client (atom false) nil nil)
> hrm, not entirely following this here. Does this actually run once per clie
As I understand, the value associated with the :client key in the result map returned by this function is used to instantiate all clients for the. So, having that atom is the way to store a state between those invocations -- the very first client which sets the atom creates the table, all other clients just open already existing table when jepsen calls setup.
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/src/main/clojure/jepsen/kudu/util.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/util.clj:
PS16, Line 31: reduce (fn [col e] (conj col e)) [] components)
> can you explain this to me? not following. isn't this equivalent to (reduce
Yes, you are right. It seems I wrote this code in an iterative way and missed the point of simplifying it in the very end :)
The sequence should not be vec-ified, using list here is OK as well.
Basically, it should be just (str/join "/" components)
Line 351: ;; Copy appropriate binaries to the node.
> can we add some kind of check before copying that the binaries we are going
Good idea, but I would like that to work also with binaries from other architecture put into the build/latest/bin: e.g., I use my OS X laptop to copy pre-build Debian/Ubuntu binaries into the DB test nodes.
I'll think how to achieve that.
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/src/utils/kudu_test_runner.clj
File java/kudu-jepsen/src/utils/kudu_test_runner.clj:
PS16, Line 82: {:masters (:master-nodes options)
: :tservers (:tserver-nodes options)
: :ssh-key-path (:ssh-key-path options)
: :iter-num (:iter-num options)}))
> seems odd to be mapping the 'options' map to new keys here, and then again
ok, passing those transparently would make more sense, yes.
PS16, Line 88: def cmd-line-opts (get-cmd-line-opts))
: (def private-key-path (:ssh-key-path cmd-line-opts))
: (def iter-num (:iter-num cmd-line-opts))
: (def test-opts (dissoc cmd-line-opts :ssh-key-path :iter-num))
> would 'let' be better here?
Unfortunately, the call of the (jepsen.kudu-test/instatiate-all-kudu-tests test-opts) does not work with locals. So, test-opts should be global, and the rest except iter-num. In that case I think it's better to keep them all global.
--
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: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
Patch Set 12:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj:
Line 1: (ns jepsen.kudu
> nit: need license headers on this and all other files in this commit
Done
Line 28: (def flags ["--fs_wal_dir=/var/lib/kudu/master"
> eastwood lint says this is a "def-in-def" which is bad (since it makes 'fla
Done
PS12, Line 32: (when (> (count (:masters test)) 1)
: (conj flags (str "--master_addresses=" (concatenate-addresses (:masters test)))))
> confused by this - it seems like a no-op, since the result of the (when...)
Good catch -- yes, indeed. That's a bug. Fixed.
Line 46: (def ntp-common-opts ["statistics loopstats peerstats clockstats"
> could you 'slurp' these from a resource file instead?
Done
PS12, Line 64: masters
> I don't think NTP uses the term 'masters'. Probably better to say 'servers'
Done
Line 66: [(str "server " (name (first masters))
> shouldn't this use all of the configured masters, not just the first? eg so
Done
PS12, Line 69: (defn db
: []
: "Apache Kudu."
> src/main/clojure/jepsen/kudu.clj:69:7: misplaced-docstrings: Possibly mispl
Done
PS12, Line 151: (into []
> https://github.com/bbatsov/clojure-style-guide says to use (vec ...) instea
Done
http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj:
Line 37: ([name type] (.build (.key (new ColumnSchema$ColumnSchemaBuilder name, type) false)))
> can this be defined as (column-schema name type false) to avoid redundancy?
Good idea! Done.
Line 38: ([name type key?] (.build (.key (new ColumnSchema$ColumnSchemaBuilder name, type) key?))))
> how about:
Done
PS12, Line 51: columns (.getColumns (.getSchema row-result)
> again maybe the java programmer in me but I think (-> row-result .getSchema
Done
PS12, Line 55: case (.ordinal (.getType column))
: ;; Clojure transforms enums in literals
: ;; so we have to use ordinals :(
> what about using cond instead?
Good idea -- I replaced that with condp and it works.
PS12, Line 71: ->t
> the style guide I'm looking at says to use -> for "conversion functions" bu
Done: renamed into 'drain-scanner-into-tuples'
--
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has submitted this change and it was merged.
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was
failing) and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other assorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
Restrictions:
1. Kudu nodes should run recent Debian/Ubuntu Linux distro
(that's due to the internal Jepsen's restrictions).
2. The 'kudu-master', 'kudu-tserver' and 'kudu' binaries in
the $KUDU_HOME/build/latest/bin should be built for the OS/distro
running on the Kudu cluster nodes.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
Reviewed-on: http://gerrit.cloudera.org:8080/5492
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/resources/ntp.conf.common
A java/kudu-jepsen/resources/ntp.conf.server
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
15 files changed, 1,557 insertions(+), 0 deletions(-)
Approvals:
David Ribeiro Alves: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/5492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
Gerrit-PatchSet: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
Patch Set 18:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/5492/18//COMMIT_MSG
Commit Message:
PS18, Line 10: failing)
> nit: wrap this
Done
PS18, Line 15: unsorted
> you mean asorted, no? (i.e. miscellaneous, not out of order)
yup, I meant 'assorted'. Fixed.
PS18, Line 25: All Jepsen control operations on the DB nodes (i.e. Kudu master and
: tserver nodes) are run via SSH. The private SSH key should be set prior
: to running the test:
:
: 1. The public part of the SSH key should be added into the
: 'authorized_keys' file for the root user on all cluster nodes.
:
: 2. The private part of the SSH key should be provided to the test
: either by:
: * adding the key into the SSH agent on the control node
: * specifying the path to the key via 'sshKeyPath' property
:
: Having the Kudu cluster provisioned and SSH keys deployed, to run
: the tests against the cluster with master node m0 and tserver nodes
: {t0..t4}, build the Kudu project from sources and then execute
: the following in the $KUDU_HOME/java/kudu-jepsen directory:
:
: mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
:
: after bulding the top-level project with
:
: mvn clean compile test-compile -Pjepsen
> don't think this info needs to be here. Is it in the README?
Yep, that also present in the README.adoc, in a little different format. Will remove.
http://gerrit.cloudera.org:8080/#/c/5492/18/java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj:
PS18, Line 87: into
> s/into/to
Done
http://gerrit.cloudera.org:8080/#/c/5492/18/java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj:
PS18, Line 99: but no server sees
: the *same* majority as any other.
> is there a guarantee that the majorities are different, or is it just likel
This is a good question!
That part I didn't check -- just took the jepsen's word for it. These nemeses are modeled after the originals in $JEPSEN_GIT_ROOT/jepsen/src/jepsen/nemesis.clj, but they are acting within the subset of nodes where tservers are running (master nodes are not affected). Probably, there is a better way to achieve that, but since I'm not so good at Clojure yet, that's what I came up with :)
http://gerrit.cloudera.org:8080/#/c/5492/18/java/kudu-jepsen/src/main/clojure/jepsen/kudu/util.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/util.clj:
PS18, Line 34: 1.2.0
> isn't this 1.3.0-SNAPSHOT or whatever now?
Well, it's repo pkg version, so for the latest snapshot from the main trunk it should be just an empty string. Will update, thank you for pointing at this!
http://gerrit.cloudera.org:8080/#/c/5492/18/java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
File java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj:
PS18, Line 53: (def register-test-configs
: [
: {:suffix "tserver-random-halves"
: :nemesis '(kn/tserver-partition-random-halves)}
: {:suffix "tserver-majorities-ring"
: :nemesis '(kn/tserver-partition-majorities-ring)}
: {:suffix "kill-restart-2-tservers"
: :nemesis '(kn/kill-restart-tserver (comp (partial take 2) shuffle))}
: {:suffix "kill-restart-3-tservers"
: :nemesis '(kn/kill-restart-tserver (comp (partial take 3) shuffle))}
: {:suffix "kill-restart-all-tservers"
: :nemesis '(kn/kill-restart-tserver shuffle)}
: {:suffix "all-random-halves"
: :nemesis '(jn/partition-random-halves)}
: {:suffix "all-majorities-ring"
: :nemesis '(jn/partition-majorities-ring)}
: {:suffix "hammer-2-tservers"
: :nemesis '(kn/tserver-hammer-time (comp (partial take 2) shuffle))}
: {:suffix "hammer-3-tservers"
: :nemesis '(kn/tserver-hammer-time (comp (partial take 3) shuffle))}
: {:suffix "hammer-all-tservers"
: :nemesis '(kn/tserver-hammer-time shuffle)}
: ])
> clap clap! neato, thanks for adding all there scenarios
I'm happy that you find this useful :) We can add more scenarios down the road.
--
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: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP: Kudu Jepsen Tests - Initial Commit
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: WIP: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 4:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/.gitignore
File java/kudu-jepsen/.gitignore:
Line 1: /target
I think this shouldn't be necessary since the java/.gitignore contains target/.
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/CHANGELOG.md
File java/kudu-jepsen/CHANGELOG.md:
Line 1: # Change Log
can probably be removed?
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/LICENSE
File java/kudu-jepsen/LICENSE:
Line 1: THE ACCOMPANYING PROGRAM IS PROVIDED UNDER THE TERMS OF THIS ECLIPSE PUBLIC
Remove
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/doc/intro.md
File java/kudu-jepsen/doc/intro.md:
Line 1: # Introduction to jepsen.kudu
remove
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/pom.xml
File java/kudu-jepsen/pom.xml:
Line 56: <exclusions>
Could you document the reason for this exclusion? I wouldn't expect the client to work without the slf4j-api jar being on the classpath.
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/project.clj
File java/kudu-jepsen/project.clj:
PS1, Line 4: Eclipse Public License
ASL
http://gerrit.cloudera.org:8080/#/c/5492/4/java/kudu-jepsen/project.clj
File java/kudu-jepsen/project.clj:
Line 1: (defproject kudu "0.1.0-SNAPSHOT"
Is this file necessary? Seems like we should have either the pom.xml or project.clj, not both.
http://gerrit.cloudera.org:8080/#/c/5492/1/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
Yah, we should try and use the built in MiniCluster here, if possible. Would simplify this a lot.
PS1, Line 54: \u2028
whitespace
http://gerrit.cloudera.org:8080/#/c/5492/4/java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj:
Line 32: (when (> (count (:masters test)) 1)
Why only set the master addresses when there are more than one master? This whole thing could be simplified to look more like kudu-cfg-tserver.
PS4, Line 54: \u2028
trailing whitespace
PS4, Line 59: into []
I don't think the 'into []' is necessary, str/join should work fine over a seq.
user=> (def foo ["foo" "bar"])
#'user/foo
user=> (def bar ["baz" "bazzle"])
#'user/bar
user=> (clojure.string/join "\n" (concat foo bar))
"foo\nbar\nbaz\nbazzle"
PS4, Line 59: efn
this could be a def
PS4, Line 63: into []
Same.
http://gerrit.cloudera.org:8080/#/c/5492/4/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj:
Line 40: (defn kv-table-options
Any reason not to use hash partitioning? Would make this a lot easier, and it's more realistic for a K/V workload.
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
Patch Set 22: Code-Review+2
--
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 12:
(1 comment)
BTW, what would be the best way to update this patch given the fact there are subsequent ones?
I would suggest to fix only issues which are not addressed in the subsequent patches. Does it sound reasonable?
http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj:
PS12, Line 123: (meh (->> (c/exec :service :kudu-master :stop)))
: (meh (->> (c/exec :rm :-rf "/var/lib/kudu/master"))))
:
: (when (.contains (:tservers test) node)
: (info node "Stopping Kudu Tablet Server")
: (meh (->> (c/exec :service :kudu-tserver :stop)))
: (meh (->> (c/exec :rm :-rf "/var/lib/kudu/tserver")))))
> src/main/clojure/jepsen/kudu.clj:123:16: suspicious-expression: ->> called
The suspect is gone in the subsequent patch (review item 5500).
--
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#19).
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was failing)
and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other unsorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH. The private SSH key should be set prior
to running the test:
1. The public part of the SSH key should be added into the
'authorized_keys' file for the root user on all cluster nodes.
2. The private part of the SSH key should be provided to the test
either by:
* adding the key into the SSH agent on the control node
* specifying the path to the key via 'sshKeyPath' property
Having the Kudu cluster provisioned and SSH keys deployed, to run
the tests against the cluster with master node m0 and tserver nodes
{t0..t4}, build the Kudu project from sources and then execute
the following in the $KUDU_HOME/java/kudu-jepsen directory:
mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
after bulding the top-level project with
mvn clean compile test-compile -Pjepsen
Restrictions:
1. Kudu nodes should run recent Debian/Ubuntu Linux distro
(that's due to the internal Jepsen's restrictions).
2. The 'kudu-master', 'kudu-tserver' and 'kudu' binaries in
the $KUDU_HOME/build/latest/bin should be built for the OS/distro
running on the Kudu cluster nodes.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/resources/ntp.conf.common
A java/kudu-jepsen/resources/ntp.conf.server
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
15 files changed, 1,544 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/19
--
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: 19
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [jepsen] initial patch from David
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#4).
Change subject: [jepsen] initial patch from David
......................................................................
[jepsen] initial patch from David
WIP: 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.
WIP as this still needs some cleanup/minimal docs but
should be reasonably ready for a first review iteration.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/CHANGELOG.md
A java/kudu-jepsen/LICENSE
A java/kudu-jepsen/README.md
A java/kudu-jepsen/doc/intro.md
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/project.clj
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
15 files changed, 784 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/4
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#17).
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was failing)
and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other unsorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH. The private SSH key should be set prior
to running the test:
1. The public part of the SSH key should be added into the
'authorized_keys' file for the root user on all cluster nodes.
2. The private part of the SSH key should be provided to the test
either by:
* adding the key into the SSH agent on the control node
* specifying the path to the key via 'sshKeyPath' property
Having the Kudu cluster provisioned and SSH keys deployed, to run
the tests against the cluster with master node m0 and tserver nodes
{t0..t4}, build the Kudu project from sources and then execute
the following in the $KUDU_HOME/java/kudu-jepsen directory:
mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
after bulding the top-level project with
mvn clean compile test-compile -Pjepsen
Restrictions:
1. Kudu nodes should run recent Debian/Ubuntu Linux distro
(that's due to the internal Jepsen's restrictions).
2. The 'kudu-master', 'kudu-tserver' and 'kudu' binaries in
the $KUDU_HOME/build/latest/bin should be built for the OS/distro
running on the Kudu cluster nodes.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/resources/ntp.conf.common
A java/kudu-jepsen/resources/ntp.conf.server
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
15 files changed, 1,479 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/17
--
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: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [jepsen] initial patch from David
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [jepsen] initial patch from David
......................................................................
Patch Set 4:
(1 comment)
David, it seems I messed up with Change-Id while picking your patch as a base for next jepsen changes. So, accidentally updated comment for your patch. I'll return it back.
Sorry for the mess.
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj:
PS1, Line 30: (:table-name test)
nit: consider introducing a variable for that in this scope: it's referenced twice in this context.
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#18).
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was failing)
and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other unsorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH. The private SSH key should be set prior
to running the test:
1. The public part of the SSH key should be added into the
'authorized_keys' file for the root user on all cluster nodes.
2. The private part of the SSH key should be provided to the test
either by:
* adding the key into the SSH agent on the control node
* specifying the path to the key via 'sshKeyPath' property
Having the Kudu cluster provisioned and SSH keys deployed, to run
the tests against the cluster with master node m0 and tserver nodes
{t0..t4}, build the Kudu project from sources and then execute
the following in the $KUDU_HOME/java/kudu-jepsen directory:
mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
after bulding the top-level project with
mvn clean compile test-compile -Pjepsen
Restrictions:
1. Kudu nodes should run recent Debian/Ubuntu Linux distro
(that's due to the internal Jepsen's restrictions).
2. The 'kudu-master', 'kudu-tserver' and 'kudu' binaries in
the $KUDU_HOME/build/latest/bin should be built for the OS/distro
running on the Kudu cluster nodes.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/resources/ntp.conf.common
A java/kudu-jepsen/resources/ntp.conf.server
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
15 files changed, 1,507 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/18
--
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: 18
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 (#12).
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, 563 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/12
--
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
Patch Set 16:
(20 comments)
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/.gitignore
File java/kudu-jepsen/.gitignore:
Line 1: # Ignore files containing information on prior Leiningen runs/sessions.
add apache header
Line 5: /store
why do you need '/store' instead of just 'store' or 'store/'?
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/README.adoc
File java/kudu-jepsen/README.adoc:
PS16, Line 57: The Kudu master and tserver nodes should be created prior to running the test:
: the tests does not create those itself
can you clarify whether you just mean that the machines should be created, or whether there should actually be a tserver/master installed?
PS16, Line 147: liear
typo
PS16, Line 152: activisation
activation
PS16, Line 156: correspondning
corresponding
PS16, Line 158: should under
should be under
PS16, Line 162: The Jepsen code is distributed under the Eclipse Public License either
: version 1.0 or (at your option) any later version.
If we're not shipping Jepsen itself, I dont think it's necessary to specify the license of the Maven dependency
Line 165: The kudu-jepsen is licensed under the Apache License, Version 2.0
this should be implicitly covered by the top-level LICENSE file we already have in the repo
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/resources/kudu.flags
File java/kudu-jepsen/resources/kudu.flags:
Line 1: # This overrides all flags in a flag file
nit: add license (or add to rat excludes if not possible)
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/resources/ntp.conf.common
File java/kudu-jepsen/resources/ntp.conf.common:
Line 1: tinker panic 0
same (license)
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj:
PS16, Line 92: do (let [rr-iter (.nextRows scanner)]
: (while (.hasNext rr-iter)
: (do
are these two 'do's necessary given the single-expression loop bodies?
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj:
PS16, Line 34: respond
response
PS16, Line 35: respond
response
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj:
PS16, Line 71: client (atom false) nil nil)
hrm, not entirely following this here. Does this actually run once per client? If so, do you want them to be sharing an atom? it seems like it ends up creating a separate atom per client, in which case the locking and CAS on it wouldn't do anything?
PS16, Line 73: ;; (count (:tservers test))
: ;; :num_replicas 1
remove this commented-out code
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/src/main/clojure/jepsen/kudu/util.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/util.clj:
PS16, Line 31: reduce (fn [col e] (conj col e)) [] components)
can you explain this to me? not following. isn't this equivalent to (reduce conj [] components)? And isn't that equivalent to just (vec components)?
(and not sure why it has to be 'vec'ified anyway)?
Line 351: ;; Copy appropriate binaries to the node.
can we add some kind of check before copying that the binaries we are going to copy are not a shared-lib build? eg run 'ldd' and check that it doesn't have dependencies in the build dir?
http://gerrit.cloudera.org:8080/#/c/5492/16/java/kudu-jepsen/src/utils/kudu_test_runner.clj
File java/kudu-jepsen/src/utils/kudu_test_runner.clj:
PS16, Line 82: {:masters (:master-nodes options)
: :tservers (:tserver-nodes options)
: :ssh-key-path (:ssh-key-path options)
: :iter-num (:iter-num options)}))
seems odd to be mapping the 'options' map to new keys here, and then again map to new keys on line 88-91 below
PS16, Line 88: def cmd-line-opts (get-cmd-line-opts))
: (def private-key-path (:ssh-key-path cmd-line-opts))
: (def iter-num (:iter-num cmd-line-opts))
: (def test-opts (dissoc cmd-line-opts :ssh-key-path :iter-num))
would 'let' be better 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: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 12:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/5492/12/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:
PS12, Line 358: /opt/apache-maven-3.3.9/bin
Are we ready to migrate to the new maven to build everything Java?
If doing so, why not to transition to Java8 for everything as well?
From the other side, if we are using Java8 and just for kudu-jepsen, why not to leave it as a special profile/configuration? At least, that works right now, and since we are not about to run kudu-jepsen as a pre-commit verification test, having a separate profile for kudu-jepsen makes sense IMO.
PS12, Line 369: /opt/apache-maven-3.3.9/bin/mvn
If this path is used twice already, may be introduce a variable for that and use it everywhere?
http://gerrit.cloudera.org:8080/#/c/5492/12/build-support/jenkins/toolchains.xml
File build-support/jenkins/toolchains.xml:
PS12, Line 13:
an extra-space
--
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#13).
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was failing)
and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other unsorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
All Jepsen control operations on the DB nodes (i.e. Kudu master and
tserver nodes) are run via SSH. The private SSH key should be set prior
to running the test:
1. The public part of the SSH key should be added into the
'authorized_keys' file for the root user on all cluster nodes.
2. The private part of the SSH key should be provided to the test
either by:
* adding the key into the SSH agent on the control node
* specifying the path to the key via 'sshKeyPath' property
Having the Kudu cluster provisioned and SSH keys deployed, to run
the tests against the cluster with master node m0 and tserver nodes
{t0..t4}, build the Kudu project from sources and then execute
the following in the $KUDU_HOME/java/kudu-jepsen directory:
mvn clojure:run -DmasterNodes=m0 -DtserverNodes="t0,t1,t2,t3,t4"
after bulding the top-level project with
mvn compile -Pjepsen
Restrictions:
1. The Kudu cluster should consist of Linux machines of the same
architecture and OS distro as the machine where the Kudu
kudu-master, kudu-tserver, and the 'kudu' CLI utility are built.
2. As for now, the kudu-jepsen test runs on recent Debian distros
only (jepsen supports only Debian Linux).
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
13 files changed, 1,387 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/13
--
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: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#22).
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
[kudu-jepsen] Kudu Jepsen tests
This patch contains David's code for the initial kudu-jepsen tests
as it was before KUDU-798 was resolved (i.e. as it was when it was
failing) and additional updates/fixes:
* Extra nemeses for the read/write register linearizability test
* Run multiple test scenarios in the scope of the register test
* Starting up master server: wait for the catalog manager
* Other assorted fixes for more robust operation
The clojure code is integrated into the Kudu maven build and is compiled
along with the other projects in a separate 'jepsen' profile.
The patch also adds functionality to run the kudu-jepsen tests
from the clojure-maven-plugin. The test uses the build machine
as the Jepsen control node, running the control logic and the Kudu
Java client there.
Restrictions:
1. Kudu nodes should run recent Debian/Ubuntu Linux distro
(that's due to the internal Jepsen's restrictions).
2. The 'kudu-master', 'kudu-tserver' and 'kudu' binaries in
the $KUDU_HOME/build/latest/bin should be built for the OS/distro
running on the Kudu cluster nodes.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/README.adoc
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/resources/kudu.flags
A java/kudu-jepsen/resources/ntp.conf.common
A java/kudu-jepsen/resources/ntp.conf.server
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/main/clojure/jepsen/kudu/util.clj
A java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
A java/kudu-jepsen/src/utils/kudu_test_runner.clj
M java/pom.xml
15 files changed, 1,557 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/22
--
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 (#10).
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/10
--
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: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [jepsen] initial patch from David
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 (#3).
Change subject: [jepsen] initial patch from David
......................................................................
[jepsen] initial patch from David
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/CHANGELOG.md
A java/kudu-jepsen/LICENSE
A java/kudu-jepsen/README.md
A java/kudu-jepsen/doc/intro.md
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/project.clj
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
15 files changed, 784 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/3
--
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 12:
(18 comments)
http://gerrit.cloudera.org:8080/#/c/5492/12/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:
PS12, Line 369: /opt/apache-maven-3.3.9/bin/mvn
> If this path is used twice already, may be introduce a variable for that an
would prefer not to hard-code any paths to maven here. Instead we should probably just change our job config to put maven 3.3 on the path first and say that maven 3.3 is required.
http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/resources/kudu.flags
File java/kudu-jepsen/resources/kudu.flags:
PS12, Line 2: --fromenv=rpc_bind_addresses
: --fromenv=log_dir
hrm, why are these grabbed from the env?
http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj:
Line 1: (ns jepsen.kudu
nit: need license headers on this and all other files in this commit
Line 17: ;; TODO allow to replace the binaries with locally built ones
is this fixed in a later patch in the series? definitely seems fairly important to fix so that the tests in the repo actually test the code in the repo (especially given the below URL isn't publicly accessible afaik)
Line 28: (def flags ["--fs_wal_dir=/var/lib/kudu/master"
eastwood lint says this is a "def-in-def" which is bad (since it makes 'flags' a global)
PS12, Line 32: (when (> (count (:masters test)) 1)
: (conj flags (str "--master_addresses=" (concatenate-addresses (:masters test)))))
confused by this - it seems like a no-op, since the result of the (when...) expression isn't assigned anywhere, and 'flags' isn't mutated by the (conj)
Line 46: (def ntp-common-opts ["statistics loopstats peerstats clockstats"
could you 'slurp' these from a resource file instead?
PS12, Line 64: masters
I don't think NTP uses the term 'masters'. Probably better to say 'servers' or 'ntp-servers' or something.
Line 66: [(str "server " (name (first masters))
shouldn't this use all of the configured masters, not just the first? eg something like:
(concat ntp-common-opts
(map #(str "server " (name %) "burst iburst ...")
masters))
PS12, Line 69: (defn db
: []
: "Apache Kudu."
src/main/clojure/jepsen/kudu.clj:69:7: misplaced-docstrings: Possibly misplaced docstring, db
src/main/clojure/jepsen/kudu.clj:69:1: unused-ret-vals: Constant value is discarded: "Apache Kudu."
PS12, Line 123: (meh (->> (c/exec :service :kudu-master :stop)))
: (meh (->> (c/exec :rm :-rf "/var/lib/kudu/master"))))
:
: (when (.contains (:tservers test) node)
: (info node "Stopping Kudu Tablet Server")
: (meh (->> (c/exec :service :kudu-tserver :stop)))
: (meh (->> (c/exec :rm :-rf "/var/lib/kudu/tserver")))))
src/main/clojure/jepsen/kudu.clj:123:16: suspicious-expression: ->> called with 1 args. (->> x) always returns x. Perhaps there are misplaced parentheses?
src/main/clojure/jepsen/kudu.clj:124:16: suspicious-expression: ->> called with 1 args. (->> x) always returns x. Perhaps there are misplaced parentheses?
src/main/clojure/jepsen/kudu.clj:128:16: suspicious-expression: ->> called with 1 args. (->> x) always returns x. Perhaps there are misplaced parentheses?
src/main/clojure/jepsen/kudu.clj:129:16: suspicious-expression: ->> called with 1 args. (->> x) always returns x. Perhaps there are misplaced parentheses?
PS12, Line 151: (into []
https://github.com/bbatsov/clojure-style-guide says to use (vec ...) instead of (into [] ...)
http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj:
Line 37: ([name type] (.build (.key (new ColumnSchema$ColumnSchemaBuilder name, type) false)))
can this be defined as (column-schema name type false) to avoid redundancy?
Line 38: ([name type key?] (.build (.key (new ColumnSchema$ColumnSchemaBuilder name, type) key?))))
how about:
([name type key?] (->
(new ColumnSchema$ColumnSchemaBuilder name, type)
(.key key?)
.build))
to avoid the nesting?
PS12, Line 51: columns (.getColumns (.getSchema row-result)
again maybe the java programmer in me but I think (-> row-result .getSchema .getColumns) is more readable?
PS12, Line 55: case (.ordinal (.getType column))
: ;; Clojure transforms enums in literals
: ;; so we have to use ordinals :(
what about using cond instead?
(let [name (.getName column)
type (.getType column)
value (cond
(= type Type/INT32) (.getInt row-result idx)
(= type Type/INT64) (.getLong row-result idx)
...)])
PS12, Line 71: ->t
the style guide I'm looking at says to use -> for "conversion functions" but I don't think this is such a function.
http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj:
Line 11: (defn replace-nodes
not really understading this function
--
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] WIP: Kudu Jepsen Tests - Initial Commit
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 (#2).
Change subject: WIP: Kudu Jepsen Tests - Initial Commit
......................................................................
WIP: 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.
WIP as this still needs some cleanup/minimal docs but
should be reasonably ready for a first review iteration.
Change-Id: I590c6e78840304b3131666c7037ff9a08dc77dea
---
A java/kudu-jepsen/.gitignore
A java/kudu-jepsen/CHANGELOG.md
A java/kudu-jepsen/LICENSE
A java/kudu-jepsen/README.md
A java/kudu-jepsen/doc/intro.md
A java/kudu-jepsen/pom.xml
A java/kudu-jepsen/project.clj
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
15 files changed, 784 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/5492/2
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] WIP: Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: WIP: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 1:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/.gitignore
File java/kudu-jepsen/.gitignore:
Line 1: /target
> I think this shouldn't be necessary since the java/.gitignore contains targ
Done
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/CHANGELOG.md
File java/kudu-jepsen/CHANGELOG.md:
Line 1: # Change Log
> can probably be removed?
Done
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/LICENSE
File java/kudu-jepsen/LICENSE:
Line 1: THE ACCOMPANYING PROGRAM IS PROVIDED UNDER THE TERMS OF THIS ECLIPSE PUBLIC
> Remove
Done
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/doc/intro.md
File java/kudu-jepsen/doc/intro.md:
Line 1: # Introduction to jepsen.kudu
> remove
Done
http://gerrit.cloudera.org:8080/#/c/5492/1/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
> Yah, we should try and use the built in MiniCluster here, if possible. Wou
That would be great to be able to use MiniCluster for this test, but that would not work with jepsen. Or it would be necessary to re-write the most control part of it. So, probably not this time.
This comment was more about using the fresh built, not packaged binaries for the test. That's done in one of my follow-up patches.
PS1, Line 54: \u2028
> whitespace
Done
http://gerrit.cloudera.org:8080/#/c/5492/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj:
PS1, Line 30: (:table-name test)
> nit: consider introducing a variable for that in this scope: it's reference
That's done in https://gerrit.cloudera.org/#/c/5500/
--
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: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 12:
> Hrm, it sounds like the subsequent patch rewrites/improves a lot of
> what's in this one. Why not squash the patches so that reviewers
> don't have to spend time looking at stuff that's already been
> removed/changed?
OK, that sounds good to me -- I hope David does not mind. Originally we have several patches because we were working on different things independently, but now we can merge that.
--
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 12:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/5492/12/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:
PS12, Line 369: /opt/apache-maven-3.3.9/bin/mvn
> would prefer not to hard-code any paths to maven here. Instead we should pr
As I see it, this file should not be modified for the first iteration -- we discussed that with Adar and the proposed solution would be just build the kudu-jepsen module only for special profile -- that's to use in the script for the Jenkins job (that's already done).
http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/resources/kudu.flags
File java/kudu-jepsen/resources/kudu.flags:
PS12, Line 2: --fromenv=rpc_bind_addresses
: --fromenv=log_dir
> hrm, why are these grabbed from the env?
This is gone -- in the subsequent change I removed that:
https://gerrit.cloudera.org/#/c/5500
http://gerrit.cloudera.org:8080/#/c/5492/12/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
> is this fixed in a later patch in the series? definitely seems fairly impor
That's done in my subsequent patch:
https://gerrit.cloudera.org/#/c/5500
http://gerrit.cloudera.org:8080/#/c/5492/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj:
Line 11: (defn replace-nodes
> not really understading this function
It seems like a hack to me too. As I understand, this is a hack to filter tablet server nodes from the array of all nodes (the latter contains master as well). I removed this hack in my subsequent patch (review item 5500).
--
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5492/9/build-support/jenkins/toolchains.xml
File build-support/jenkins/toolchains.xml:
Line 4: <!-- JDK toolchains -->
> OK, but effect does this file have? Would be nice to explain in this commen
this is my attempt at getting the build to use jdk 8 just for the jepsen module. it requires pushing this stuff until it works. Will update the file once its finished.
--
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: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Kudu Jepsen Tests - Initial Commit
......................................................................
Patch Set 12:
Hrm, it sounds like the subsequent patch rewrites/improves a lot of what's in this one. Why not squash the patches so that reviewers don't have to spend time looking at stuff that's already been removed/changed?
--
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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: <properties>
: <skipTests>true</skipTests>
: </properties>
> Why this? Add an XML comment to explain.
Done
Line 31:
> Remove this.
Done
Line 49: <version>0.1.3</version>
> 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 <properties/> section. Done that.
Line 56: <exclusions>
> Why the slf4j-api exclusion here and below? Add a comment explaining.
Done
Line 89: <extensions>true</extensions>
> 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: <!-- Build the jepsen test for Kudu.
> Why hide the jepsen tests behind a disabled-by-default Maven profile? Is it
yeah, I agree with Adar here. these should be built by default, even if the tests are not run. I removed this.
--
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 <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [kudu-jepsen] Kudu Jepsen tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: [kudu-jepsen] Kudu Jepsen tests
......................................................................
Patch Set 22:
> (7 comments)
>
> final q: does this still work with docker instances?
Last time I checked that before I had robust setup with Jenkins. That I'll verify today.
--
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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: <!-- Build the jepsen test for Kudu.
> yeah, I agree with Adar here. these should be built by default, even if the
yeah, then wait until the default maven and java get updated on Jenkins slaves, so the new maven and JAVA8 used -- otherwise the build fails.
--
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 <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Kudu Jepsen Tests - Initial Commit
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>