Hao Hao 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: (9 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? Updated to run 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 Done 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? Done http://gerrit.cloudera.org:8080/#/c/9526/5/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@19 PS5, Line 19: "Set test" > Yep, that would be helpful. Trying to do that, but not many comments were presented there and most of the comments do not apply to this variation anymore. So added more comments. Let me know if it is still not clear. 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 potential Done 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 dow Good point, updated. 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. Done 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 > nit: another option would be adding Clojure-style comment, if it makes any Done 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? Done -- 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, 30 Apr 2018 22:50:43 +0000 Gerrit-HasComments: Yes
