Alexey Serbin 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 12: (10 comments) http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@13 PS9, Line 13: alidates that the total : row count of the table read by the client is always greater than or : equal to the count of previo > If the client uses READ_LATEST on a non-leader replica, this could be broke Ah, I see, that makes sense. http://gerrit.cloudera.org:8080/#/c/9526/12//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9526/12//COMMIT_MSG@12 PS12, Line 12: : Actually, I meant to replace comma with the column, i.e. update this like ... operations: 'count' ..., 'add' ... http://gerrit.cloudera.org:8080/#/c/9526/9/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/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@38 PS9, Line 38: ;; [{:type :invoke, :f :add, :value 1, :process 0} : ;; {:type :ok, :f :add, :value 1, :process 0} : ;; {:type :invoke, :f :add, :value 2, :process 0} : ;; {:type :ok, :f :add, :value 2, :process 0} : ;; {:type :invoke, :f :count, :value nil, :process 0} : ;; {:type :ok, :f :count, :value 2, :process 0} : ;; {:type :invoke, :f :count, :value nil, :process 1} : ;; {:type :ok, :f :count, :value 2, :process 1} : ;; {:type :invoke, :f :add, :value 13, :process 1} : ;: {:type :ok, :f :add, :value 13, :process 1} : ;; {:type :invoke, :f :add, :value 22, :process 0} : ;; {:type :ok, :f :add, :value 22, :process 0} : ;; {:type :invoke, :f :count, :value nil, :process 0} : ;; {:type :ok, :f :count, :value 2, :process 0} : ;; {:type :invoke, :f :add, :value 250, :process 1} : ;; {:type :ok, :f :add, :value 250, :process 1}] : ;; > Right, the expected count output should be 3. Updated it in the latest vers For some reason I don't see the explanatory comment in PS12. Do I miss something? I was expected to see something like ... process 0 has successfully inserted 3 rows but only 2 rows have been read ... http://gerrit.cloudera.org:8080/#/c/9526/12/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/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@67 PS12, Line 67: [clojure.core.reducers :as r] Is this used in this file at all? http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@68 PS12, Line 68: [clojure.set :as set] Is this used in this file at all? http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@72 PS12, Line 72: ;; The add operation to be performed by the processes. Maybe, make it Clojure-style documentation? http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@79 PS12, Line 79: ;; The count operation to be performed by the processes. Ditto http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@82 PS12, Line 82: count-by-set BTW, what happens if a write operation fails? Does the test signal an assertion failure or it is interpreted just as some other failure? E.g., in case of current jepsen test for the register model, we have not an assertion failure (that would signal about consistency anomaly), but rather an error in the summary, and using that information it's easy to tell whether it's some generic error or consistency-related assertion. http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@87 PS12, Line 87: knossos.op Why not just 'op'? The require directive above introduced op for knossos.op already, no? http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@92 PS12, Line 92: knossos.op ditto -- 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: 12 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: Wed, 09 May 2018 19:48:04 +0000 Gerrit-HasComments: Yes
