Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RethinkDB Test #70

Merged
merged 5 commits into from Nov 3, 2015
Merged

RethinkDB Test #70

merged 5 commits into from Nov 3, 2015

Conversation

@mlucy
Copy link
Contributor

mlucy commented Jul 29, 2015

Requires the version of clj-rethinkdb at https://github.com/mlucy/clj-rethinkdb/tree/mlucy_error_types .

@mlucy

This comment has been minimized.

Copy link
Contributor Author

mlucy commented Jul 29, 2015

I've never written clojure before, so apologies in advance if it's unidiomatic.

@@ -0,0 +1,17 @@
(defproject mongodb "0.1.0-SNAPSHOT"
:description "RethinkDB Jepsen Tests"
; :url "http://github.com/rethinkdb/jepsen"

This comment has been minimized.

Copy link
@mlucy

mlucy Jul 29, 2015

Author Contributor

What should go here do you think? All the other toplevel directories had example.com/FIXME here.

This comment has been minimized.

Copy link
@aphyr

aphyr Sep 26, 2015

Collaborator

You can use the jepsen repo URL if you like, but it's not mandatory. I'm more concerned that the project name is "mongodb". I suggest "jepsen.rethinkdb".

;; TODO: detect server failing to start.
(c/ssh* {:cmd "killall rethinkdb bash"})
(c/ssh* {:cmd "
# Add a 100ms delay to help find edge cases.

This comment has been minimized.

Copy link
@mlucy

mlucy Jul 29, 2015

Author Contributor

What's your policy on code that's sometimes useful when fiddling but isn't being actively used? Should I just cut these commented-out lines?

This comment has been minimized.

Copy link
@aphyr

aphyr Sep 26, 2015

Collaborator

# isn't a comment in Clojure--did you mean ;? I'm fine with commented out stuff though. :)

This comment has been minimized.

Copy link
@aphyr

aphyr Sep 26, 2015

Collaborator

Oh wait this is all one massive bash script? I'm sorry, I didn't realize. Um... how about moving this out to resources/setup.sh, and slurping it in via (slurp (io/resource "setup.sh"))?

;; This is the only safe read/write mode. Changing either of
;; these (or turning on soft durability) may produce a
;; non-linearizable history.
(test- "document {:write_acks 'majority' :read_mode 'majority'}"

This comment has been minimized.

Copy link
@mlucy

mlucy Jul 29, 2015

Author Contributor

I was in fact able to get a non-linearizable history changing these, so the test is actually testing something.

This comment has been minimized.

Copy link
@aphyr

aphyr Sep 26, 2015

Collaborator

Cool!

@@ -0,0 +1,135 @@
(ns rdb.core

This comment has been minimized.

Copy link
@aphyr

aphyr Sep 26, 2015

Collaborator

For inclusion in the main Jepsen repo, let's call this jepsen.rethinkdb

nohup /usr/bin/rethinkdb \\
--bind all \\
-n `hostname` \\
-j n1:29015 -j n2:29015 -j n3:29015 -j n4:29015 -j n5:29015 \\

This comment has been minimized.

Copy link
@aphyr

aphyr Sep 26, 2015

Collaborator

These should be parameterized by (:nodes test)

@aphyr

This comment has been minimized.

Copy link
Collaborator

aphyr commented Sep 26, 2015

Things look pretty good to me! Just a few minor namespace changes and some small refactors and we'll be good to merge. Thank you!

@mlucy

This comment has been minimized.

Copy link
Contributor Author

mlucy commented Oct 22, 2015

Sorry for the delay!

I think I addressed everything you mentioned except for the namespace issue. I don't really understand how clojure package management works; I fiddled with it for a bit but couldn't figure out how to change the namespaces to rethinkdb.X without it conflicting with the rethinkdb driver package.

@mlucy

This comment has been minimized.

Copy link
Contributor Author

mlucy commented Oct 22, 2015

(To clarify a bit, I couldn't figure out how to e.g. make the first like of core.clj be (ns rethinkdb.core without the [rethinkdb.core :refer [connect close]] on line 22 breaking.)

@aphyr

This comment has been minimized.

Copy link
Collaborator

aphyr commented Nov 3, 2015

Don't worry about it! Thanks so much for getting all this put together. :)

aphyr added a commit that referenced this pull request Nov 3, 2015
@aphyr aphyr merged commit 26c494c into jepsen-io:master Nov 3, 2015
aphyr added a commit that referenced this pull request Aug 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.