David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/9526 )
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode ...................................................................... Patch Set 5: (7 comments) http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/build.gradle File java/kudu-jepsen/build.gradle: http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/build.gradle@55 PS5, Line 55: def scanMode = propertyWithDefault("scanMode", "READ_AT_SNAPSHOT") hum, why is this a global property? don't we want to test both modes? http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/pom.xml File java/kudu-jepsen/pom.xml: http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/pom.xml@43 PS5, Line 43: <scanMode>READ_AT_SNAPSHOT</scanMode> same question http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj File java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj: http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@19 PS5, Line 19: "Set test" could you point the original cockroachdb impl? http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@130 PS5, Line 130: (using READ_YOUR_WRITES : ;; mode) this doesn't need to be exclusive to the RYW mode right? we could potentially run this test with READ_AT_SNAPSHOT http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@132 PS5, Line 132: successfully writes performed by that client. didn't we establish that we also need to check that the count didn't go down from the previous reads from that client? (RYR) http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@133 PS5, Line 133: (defn sets-test thanks for adding the docs, it helps a lot. nit: maybe move this to the top so that the explanation of how it works is the first thing a reader sees? http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj File java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj: http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj@113 PS5, Line 113: AsyncKuduScanner$ReadMode/READ_YOUR_WRITES this should be a param, no? -- To view, visit http://gerrit.cloudera.org:8080/9526 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c Gerrit-Change-Number: 9526 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 23 Apr 2018 19:17:18 +0000 Gerrit-HasComments: Yes
