You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2018/03/07 02:56:07 UTC
[kudu-CR] [WIP] updated to use READ YOUR WRITES scan mode for Jepsen tests
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9526
Change subject: [WIP] updated to use READ_YOUR_WRITES scan mode for Jepsen tests
......................................................................
[WIP] updated to use READ_YOUR_WRITES scan mode for Jepsen tests
This patches updated the Jespen tests to use READ_YOUR_WRITES scan mode.
Also, in this change every Jepsen worker will operate in a Kudu client,
instead of sharing one Kudu client for all workers.
I ran the Jepesen test with this change and it passed. The log is
available: https://drive.google.com/file/d/1jW1CeGjB9AdnJZ2XbhJrf0IyOwXnKC_p/view?usp=sharing
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
2 files changed, 14 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/1
--
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: newchange
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 10:
> Uploaded patch set 10.
I think the test would be more robust if we were deleting the inserted values from other clients (only those which they inserted).
My concern is that the way how the jepsen test for RYW modes is implemented now does not cover the case when other clients delete their stuff from the table.
Maybe, it's worth updating the client behavior so that every time the client tries to perform an operation, it could either (a) insert a new value (b) delete the previous value (c) update the previous value to the next value in its values space.
What do you think?
--
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: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 08 May 2018 19:26:12 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
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: deleteVote
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 13
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#10).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generates two kind of
operations, 'count', 'add'. It validatess that the total row count of
the table read by the client is always greater than or equal to the
count of previous writes by the same client, and the row count never go
down from the previous reads.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 230 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/10
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 9: Verified+1
--
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: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 02 May 2018 18:19:11 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 5:
(2 comments)
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?
Yep, that would be helpful.
Also, it would be nice to preserve the original comments in the code, if there were any.
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: another option would be adding Clojure-style comment, if it makes any sense.
--
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 <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Apr 2018 20:34:58 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 13:
> Unrelated flaky test.
Did you try to run it against some test cluster already?
In other words, do you mind sharing details on how you test these changes? Thanks!
--
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: 13
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 19:43:22 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 9:
Flaky test TabletCopyClientSessionITest.
--
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: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 02 May 2018 18:20:43 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 13: Verified+1
Unrelated flaky test.
--
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: 13
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 00:22:59 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#6).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generates two kind of
operations, 'count', 'add'. It validatess that the total row count of
the table read by the client is always greater than or equal to the
count of previous writes by the same client, and the row count never go
down from the previous reads.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
M java/kudu-jepsen/build.gradle
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
M java/kudu-jepsen/src/utils/kudu_test_runner.clj
5 files changed, 202 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/6
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 6
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [WIP] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#3).
Change subject: [WIP] A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
[WIP] A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generate two operations,
'count', 'add'. And it checks that the total row count of the table
read by the client is always greater than or equal to the count of
previous writes by the same client.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
2 files changed, 171 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/3
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 10:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/9526/8/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/8/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@59
PS8, Line 59:
> same
Done
http://gerrit.cloudera.org:8080/#/c/9526/8/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@59
PS8, Line 59: lude [test])
> why doesn't the shorthand used in the imports work ([clojure.core.reducers
Updated it in the latest patch.
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@25
PS9, Line 25: follow
> follows
Done
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@25
PS9, Line 25: valid
> valid:
Done
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@36
PS9, Line 36: invalid(
> invalid:
Done
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}]
: ;;
> why? what's the anomaly that the checker is flagging?
Because this history broke the first validation that 'the total row count of a shared table is greater than or equal to the count of successful writes performed by that client'. In L51, process 0 has successfully inserted 3 rows but only 2 rows have been read.
I will add more comment for it.
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@72
PS9, Line 72: ;; The add operation to be performed by the processes.
: (defn add-op [] (->> (range)
: (map (partial array-map
: :type :invoke
: :f :add
> Since you're talking about valid and invalid histories before this doesn't
Done
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@77
PS9, Line 77:
> docs: "The add operation to be performed by the processes"
Done
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@82
PS9, Line 82: (defn count-by-set
> docs: "The count operation to be performed by the processes"
Done
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@85
PS9, Line 85:
> you mean "and" right?
The input could be either a set of :add ops or a :count op. So I put 'or' here.
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@86
PS9, Line 86:
> same
Same as above.
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@101
PS9, Line 101:
> operations, making sure that:
Done
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@103
PS9, Line 103:
> sucessful
Done
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@104
PS9, Line 104: kv
> goes
Done
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@163
PS9, Line 163:
> rows of the table
Done
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@191
PS9, Line 191:
> counts
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: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 08 May 2018 01:11:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 09 May 2018 19:48:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 9:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@10
PS9, Line 10: unique value
unique values?
I.e., as I understand from the description, every client writes multiple values, and those values are unique across all active clients, right?
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@12
PS9, Line 12: ,
:
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@12
PS9, Line 12: 'count', 'add'
It would be nice to describe what 'count' and 'add' mean, in essence.
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@13
PS9, Line 13: than or equal to the
: count of previous writes by the same client, and the row count never go
: down from the previous reads
Is there any other mode in which that constraint would be broken?
I mean, by the way how the clients behave, I cannot imagine there would be some different result output. Do I miss anything?
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}]
: ;;
> why? what's the anomaly that the checker is flagging?
Yep, I agree with David: it would be nice to describe the problem in the output.
Is that something related to
;; {: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}
?
I.e., the expected count output should be 3 (given the history of the operations above), but the actual output was 2. Or it's something else?
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@198
PS9, Line 198: READ_YOUR_WRITES
Is there a mode that, if specified, will make this test fail at all?
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@203
PS9, Line 203: value
values
--
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: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 08 May 2018 01:33:58 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#12).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique values
in a shared table concurrently. The checker generates two kind of
operations, ':count' which counts the number of rows of the table, 'add'
which inserts a unique value to the table. It validates that the total
row count of the table read by the client is always greater than or
equal to the count of previous writes by the same client, and the row
count never go down from the previous reads.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 230 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/12
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 12
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 14:
(1 comment)
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: ;; returned from the count operation of the same process is two,
: ;; which conflicts with the validation constraints:
: ;;
: ;; [{: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}
: ;;
> My initial comment was more about having a small description of what is hap
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: 14
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 19:10:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] [WIP] updated to use READ YOUR WRITES scan mode for Jepsen tests
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9526 )
Change subject: [WIP] updated to use READ_YOUR_WRITES scan mode for Jepsen tests
......................................................................
Patch Set 1:
(4 comments)
> (5 comments)
>
> we'll also need to test RYR, which I'm not sure the register test
> can do, since it only writes a single row.
>
> cockroachdb has a good set of tests that we might want to look at:
> https://github.com/jepsen-io/jepsen/tree/master/cockroachdb
Thanks for sharing that! I will take a look.
http://gerrit.cloudera.org:8080/#/c/9526/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9526/1//COMMIT_MSG@9
PS1, Line 9: This patches updated the Jespen tests to use READ_YOUR_WRITES scan mode.
> we're supposed to have both eventually right?
Right, this is just a WIP patch. Eventually I will add an option to select which mode to use.
http://gerrit.cloudera.org:8080/#/c/9526/1//COMMIT_MSG@14
PS1, Line 14: https://drive.google.com/file/d/1jW1CeGjB9AdnJZ2XbhJrf0IyOwXnKC_p/view?usp=sharing
> thanks for sharing this.
So as discussed on slack I think the exception "Snapshot timestamp is earlier than the ancient history mark. Consider increasing the value of the configuration parameter --tablet_history_max_age_sec. Snapshot timestamp: P: 0 usec, L: 1 Ancient History Mark: P: 1520385210671765 usec, L: 0 Physical time difference: -1520385210.672s" may be caused by the safe time is not correctly advanced yet(similar to KUDU-2233). Do you think so?
http://gerrit.cloudera.org:8080/#/c/9526/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj:
http://gerrit.cloudera.org:8080/#/c/9526/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj@36
PS1, Line 36: [table-created? kclient ktable]
: (reify jepsen.client/Client
: (setup! [_ test _]
: "Create the client and the test table. Use the same Kudu client instance "
: "across all test actors to achieve timestamp propagation for all "
: "operations."
: (let [kclient (kc/sync-client (:master-addresses test))
: table-name (:table-name test)
: ktable (locking table-created?
: (when (compare-and-set! table-created? false true)
: (kc/create-table
: kclient
: table-name
: kt/kv-table-schema
: (let [ranges (:table-ranges test)
: rep-factor (:num-replicas test)]
: (if (nil? ranges)
: (kt/kv-table-options-hash rep-factor (count (:tservers test)))
: (kt/kv-table-options-range rep-factor ranges)))))
: (kc/open-table kclient table-name))]
: (client table-created? kclient ktable)))
> this is just a revert of the changes we made to have 1 client per table, vs
Yes, agree.
http://gerrit.cloudera.org:8080/#/c/9526/1/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/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj@100
PS1, Line 100: READ_YOUR_WRITES
> this will have to be a parameter
Agree.
--
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: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 07 Mar 2018 19:06:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 4:
> (2 comments)
>
> Thanks for this valiant effort. This is very good stuff, but the
> juicier bits are hard to review due to missing docs...
Thanks a lot for reviewing it. Sorry about not having enough docs. I will definitely add more. To give you more background, this new model is a variation of https://github.com/jepsen-io/jepsen/blob/master/cockroachdb/src/jepsen/cockroach/sets.clj.
--
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: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 13 Apr 2018 00:02:26 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 10:
> > Uploaded patch set 10.
>
> I think the test would be more robust if we were deleting the
> inserted values from other clients (only those which they
> inserted).
>
> My concern is that the way how the jepsen test for RYW modes is
> implemented now does not cover the case when other clients delete
> their stuff from the table.
>
> Maybe, it's worth updating the client behavior so that every time
> the client tries to perform an operation, it could either (a)
> insert a new value (b) delete the previous value (c) update the
> previous value to the next value in its values space.
>
> What do you think?
I think another point is that we are not quite covering the following from the doc:
> 5. Read your writes requires that whenever a client reads a given data item after updating it, the read returns the updated value (or a value that overwrote the previously written value).
And if we just count the values in our verification phase, we could miss updates from the same client if the others already submitted some new inserts in the middle. At least, using just counters and not verifying the client sees all _values_ it wrote keeps that pitfall open and hard to detect.
--
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: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 08 May 2018 19:45:43 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 13:
> > Unrelated flaky test.
>
> Did you try to run it against some test cluster already?
>
> In other words, do you mind sharing details on how you test these
> changes? Thanks!
Yeah, I test it with a cluster. The sets model is included in Kudu test as default. So just follow the steps in README.adoc of kudu-jepsen should be fine.
--
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: 13
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 20:53:33 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#4).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generate two operations,
'count', 'add'. And it checks that the total row count of the table
read by the client is always greater than or equal to the count of
previous writes by the same client.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
M java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
M java/kudu-jepsen/src/utils/kudu_test_runner.clj
6 files changed, 200 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/4
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 14: Code-Review+2
--
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: 14
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 20:02:02 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 9:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@12
PS9, Line 12: validatess
typo
http://gerrit.cloudera.org:8080/#/c/9526/8/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/8/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@59
PS8, Line 59:
same
http://gerrit.cloudera.org:8080/#/c/9526/8/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@59
PS8, Line 59: lude [test])
why doesn't the shorthand used in the imports work ([clojure.core.reducers :as r])?
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@25
PS9, Line 25: valid,
valid:
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@25
PS9, Line 25: follow
follows
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@36
PS9, Line 36: invalid,
invalid:
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}]
: ;;
why? what's the anomaly that the checker is flagging?
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@72
PS9, Line 72: ;; The invocation operation examples:
: ;; [{:process 0, :type :invoke, :f :count, :value nil}
: ;; {:process 1, :type :invoke, :f :add, :value 3}
: ;; {:process 1, :type :info, :f :add, :value 3}
: ;; {:process 0, :type :ok, :f :count, :value 0}]
Since you're talking about valid and invalid histories before this doesn't add much
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@77
PS9, Line 77: (defn add-op [] (->> (range)
docs: "The add operation to be performed by the processes"
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@82
PS9, Line 82: (defn count-op [] {:type :invoke, :f :count, :value nil})
docs: "The count operation to be performed by the processes"
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@85
PS9, Line 85: or
you mean "and" right?
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@86
PS9, Line 86: or
same
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@101
PS9, Line 101: n that:
operations, making sure that:
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@103
PS9, Line 103: successfully
sucessful
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@104
PS9, Line 104: o
goes
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@163
PS9, Line 163: the table
rows of the table
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@191
PS9, Line 191: count
counts
--
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: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 07 May 2018 19:37:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 13:
> Patch Set 13: Code-Review+2
Thank you very much for implementing this new Jepsen test!
It's worth checking whether David has more input on this.
--
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: 13
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 17:20:19 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#7).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generates two kind of
operations, 'count', 'add'. It validatess that the total row count of
the table read by the client is always greater than or equal to the
count of previous writes by the same client, and the row count never go
down from the previous reads.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 201 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/7
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 7
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 10:
> > > Uploaded patch set 10.
> >
> > I think the test would be more robust if we were deleting the
> > inserted values from other clients (only those which they
> > inserted).
> >
> > My concern is that the way how the jepsen test for RYW modes is
> > implemented now does not cover the case when other clients delete
> > their stuff from the table.
> >
> > Maybe, it's worth updating the client behavior so that every time
> > the client tries to perform an operation, it could either (a)
> > insert a new value (b) delete the previous value (c) update the
> > previous value to the next value in its values space.
> >
> > What do you think?
>
> I think another point is that we are not quite covering the
> following from the doc:
>
> > 5. Read your writes requires that whenever a client reads a given
> data item after updating it, the read returns the updated value (or
> a value that overwrote the previously written value).
>
> And if we just count the values in our verification phase, we could
> miss updates from the same client if the others already submitted
> some new inserts in the middle. At least, using just counters and
> not verifying the client sees all _values_ it wrote keeps that
> pitfall open and hard to detect.
As discussed offline with David and Alexey , it would be a heavy burden on the clients to justify Read-your-reads, since the client needs to track the values it wrote and the data are written by all other clients (Read-your-reads guarantees all subsequent reads to a given object "never return any previous values" regarding writes that other clients have done). Using counts as verification should be sufficient since if Read-your-writes/Read-your-reads guarantees are broken, there is a probability that the validation will catch it (e.g the count may go down).
It is important to add other operations such as delete the previous value and update the previous value in Jepsen test for a better coverage. However, these are not a good fit in this mode, as delete may affect the count that the validation relies on. Instead, adding a new model (or a variation of existing register model) would be a better option.
--
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: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 09 May 2018 01:25:30 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#5).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generate two operations,
'count', 'add'. And it checks that the total row count of the table
read by the client is always greater than or equal to the count of
previous writes by the same client.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
M java/kudu-jepsen/build.gradle
M java/kudu-jepsen/pom.xml
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
M java/kudu-jepsen/src/utils/kudu_test_runner.clj
6 files changed, 195 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/5
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#8).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generates two kind of
operations, 'count', 'add'. It validatess that the total row count of
the table read by the client is always greater than or equal to the
count of previous writes by the same client, and the row count never go
down from the previous reads.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 201 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/8
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
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: deleteVote
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
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: deleteVote
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 14
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 14: Verified+1
Unrelated flaky test.
--
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: 14
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 21:35:47 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 4:
(2 comments)
Thanks for this valiant effort. This is very good stuff, but the juicier bits are hard to review due to missing docs...
http://gerrit.cloudera.org:8080/#/c/9526/4/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/4/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@21
PS4, Line 21:
this file needs _a lot_ more comments. it's important as this is a fundamentally new checker and other folks might need to maintain this code the (defn client) part is pretty straighforward and boilerplate, but the rest needs pretty good docs (here or elsewhere)
http://gerrit.cloudera.org:8080/#/c/9526/4/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/4/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj@109
PS4, Line 109: kv-count
nit this is not necessarily only for "key/value" tables, it shouild be able to count the rows of any table right?
--
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: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 12 Apr 2018 22:21:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#11).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique values
in a shared table concurrently. The checker generates two kind of
operations, ':count' which count the number of rows of the table, 'add'
which inserts a unique value to the table. It validates that the total
row count of the table read by the client is always greater than or
equal to the count of previous writes by the same client, and the row
count never go down from the previous reads.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 230 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/11
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 13:
(8 comments)
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: c
> Actually, I meant to replace comma with the column, i.e. update this like
Done
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?
Yeah, in L 87 and below.
http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@68
PS12, Line 68: [knossos.op :as op])
> Is this used in this file at all?
Done
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?
Done
http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@79
PS12, Line 79: (defn count-op []
> Ditto
Done
http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@82
PS12, Line 82:
> BTW, what happens if a write operation fails? Does the test signal an asse
Yeah, similar to register model, if a write operation fails, it will not signal an assertion failure as that does not directly indicate a consistency anomaly.
http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@87
PS12, Line 87:
> Why not just 'op'? The require directive above introduced op for knossos.o
Done
http://gerrit.cloudera.org:8080/#/c/9526/12/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@92
PS12, Line 92:
> ditto
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: 13
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 09 May 2018 23:10:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 30 Apr 2018 22:50:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/9526/4/java/kudu-jepsen/pom.xml
File java/kudu-jepsen/pom.xml:
http://gerrit.cloudera.org:8080/#/c/9526/4/java/kudu-jepsen/pom.xml@43
PS4, Line 43: READ_AT_SNAPSHOT
> I'm not sure this should be a parameter of the overall test. Maybe, instea
Hmm, I am not quite sure. Wouldn't add an option to specify the scan mode be more clear on which scan mode may have problems when the test is failing?
http://gerrit.cloudera.org:8080/#/c/9526/4/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/4/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@21
PS4, Line 21:
> this file needs _a lot_ more comments. it's important as this is a fundamen
Done
http://gerrit.cloudera.org:8080/#/c/9526/4/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/4/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj@109
PS4, Line 109: kv-count
> nit this is not necessarily only for "key/value" tables, it shouild be able
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: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 23 Apr 2018 18:52:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [WIP] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/9526 )
Change subject: [WIP] A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
Patch Set 3:
Uploaded a WIP patch that adds the new Jepsen checker for a high level review. The integration part that actually uses this new checker is not included yet.
--
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: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 26 Mar 2018 19:31:15 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#14).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique values
in a shared table concurrently. The checker generates two kind of
operations: 'count' which counts the number of rows of the table, 'add'
which inserts a unique value to the table. It validates that the total
row count of the table read by the client is always greater than or
equal to the count of previous writes by the same client, and the row
count never go down from the previous reads.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 234 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/14
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 14
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#9).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique value
in a shared table concurrently. The checker generates two kind of
operations, 'count', 'add'. It validatess that the total row count of
the table read by the client is always greater than or equal to the
count of previous writes by the same client, and the row count never go
down from the previous reads.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 232 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/9
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 13: Code-Review+2
--
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: 13
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 17:18:36 +0000
Gerrit-HasComments: No
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 10:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@10
PS9, Line 10: unique value
> unique values?
Yeah, that's right.
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@12
PS9, Line 12: ,
> :
Done
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@12
PS9, Line 12: validatess
> typo
Done
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@12
PS9, Line 12: 'count', 'add'
> It would be nice to describe what 'count' and 'add' mean, in essence.
Done
http://gerrit.cloudera.org:8080/#/c/9526/9//COMMIT_MSG@13
PS9, Line 13: than or equal to the
: count of previous writes by the same client, and the row count never go
: down from the previous reads
> Is there any other mode in which that constraint would be broken?
If the client uses READ_LATEST on a non-leader replica, this could be broken.
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}]
: ;;
> Yep, I agree with David: it would be nice to describe the problem in the ou
Right, the expected count output should be 3. Updated it in the latest version.
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@198
PS9, Line 198:
> Is there a mode that, if specified, will make this test fail at all?
Yeah, using READ_LATEST should fail the test.
http://gerrit.cloudera.org:8080/#/c/9526/9/java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj@203
PS9, Line 203:
> values
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: 10
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 09 May 2018 01:24:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9526
to look at the new patch set (#13).
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique values
in a shared table concurrently. The checker generates two kind of
operations: 'count' which counts the number of rows of the table, 'add'
which inserts a unique value to the table. It validates that the total
row count of the table read by the client is always greater than or
equal to the count of previous writes by the same client, and the row
count never go down from the previous reads.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 231 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/9526/13
--
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: newpatchset
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 13
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 9:
(1 comment)
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}]
: ;;
> For some reason I don't see the explanatory comment in PS12. Do I miss som
My initial comment was more about having a small description of what is happening and what problem was flagged.
--
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: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 18:44:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9526 )
Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
......................................................................
A new Jepsen checker for READ_YOUR_WRITES scan mode
This patch adds a new Jepsen checker to validate READ_YOUR_WRITES scan
mode. In this validation mode, each Jepsen client writes unique values
in a shared table concurrently. The checker generates two kind of
operations: 'count' which counts the number of rows of the table, 'add'
which inserts a unique value to the table. It validates that the total
row count of the table read by the client is always greater than or
equal to the count of previous writes by the same client, and the row
count never go down from the previous reads.
Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Reviewed-on: http://gerrit.cloudera.org:8080/9526
Reviewed-by: David Ribeiro Alves <da...@gmail.com>
Tested-by: Hao Hao <ha...@cloudera.com>
---
A java/kudu-jepsen/src/main/clojure/jepsen/kudu/sets.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
M java/kudu-jepsen/src/test/clojure/jepsen/kudu_test.clj
3 files changed, 234 insertions(+), 3 deletions(-)
Approvals:
David Ribeiro Alves: Looks good to me, approved
Hao Hao: Verified
--
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: merged
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 15
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 9:
(1 comment)
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}]
: ;;
> My initial comment was more about having a small description of what is hap
Indeed, the description is missing and my prior comment on PS12 hasn't been addressed.
--
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: 9
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 19:08:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
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 13:
> > Patch Set 13: Code-Review+2
>
> Thank you very much for implementing this new Jepsen test!
>
> It's worth checking whether David has more input on this.
Thanks a lot for the code review!
--
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: 13
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 18:32:55 +0000
Gerrit-HasComments: No
[kudu-CR] [WIP] updated to use READ YOUR WRITES scan mode for Jepsen tests
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/9526 )
Change subject: [WIP] updated to use READ_YOUR_WRITES scan mode for Jepsen tests
......................................................................
Patch Set 1:
(5 comments)
we'll also need to test RYR, which I'm not sure the register test can do, since it only writes a single row.
cockroachdb has a good set of tests that we might want to look at: https://github.com/jepsen-io/jepsen/tree/master/cockroachdb
http://gerrit.cloudera.org:8080/#/c/9526/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9526/1//COMMIT_MSG@9
PS1, Line 9: This patches updated the Jespen tests to use READ_YOUR_WRITES scan mode.
we're supposed to have both eventually right?
http://gerrit.cloudera.org:8080/#/c/9526/1//COMMIT_MSG@13
PS1, Line 13: I ran the Jepesen test with this change and it passed.
the test only failed very occasionally, we'd need quite a few runs before we're confident the problem doens't reappear
http://gerrit.cloudera.org:8080/#/c/9526/1//COMMIT_MSG@14
PS1, Line 14: https://drive.google.com/file/d/1jW1CeGjB9AdnJZ2XbhJrf0IyOwXnKC_p/view?usp=sharing
thanks for sharing this.
http://gerrit.cloudera.org:8080/#/c/9526/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj:
http://gerrit.cloudera.org:8080/#/c/9526/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj@36
PS1, Line 36: [table-created? kclient ktable]
: (reify jepsen.client/Client
: (setup! [_ test _]
: "Create the client and the test table. Use the same Kudu client instance "
: "across all test actors to achieve timestamp propagation for all "
: "operations."
: (let [kclient (kc/sync-client (:master-addresses test))
: table-name (:table-name test)
: ktable (locking table-created?
: (when (compare-and-set! table-created? false true)
: (kc/create-table
: kclient
: table-name
: kt/kv-table-schema
: (let [ranges (:table-ranges test)
: rep-factor (:num-replicas test)]
: (if (nil? ranges)
: (kt/kv-table-options-hash rep-factor (count (:tservers test)))
: (kt/kv-table-options-range rep-factor ranges)))))
: (kc/open-table kclient table-name))]
: (client table-created? kclient ktable)))
this is just a revert of the changes we made to have 1 client per table, vs one per thread. eventually we'll want to be able to choose so that we test both modes.
http://gerrit.cloudera.org:8080/#/c/9526/1/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/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj@100
PS1, Line 100: READ_YOUR_WRITES
this will have to be a parameter
--
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: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 07 Mar 2018 07:26:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
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 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9526/4/java/kudu-jepsen/pom.xml
File java/kudu-jepsen/pom.xml:
http://gerrit.cloudera.org:8080/#/c/9526/4/java/kudu-jepsen/pom.xml@43
PS4, Line 43: READ_AT_SNAPSHOT
I'm not sure this should be a parameter of the overall test. Maybe, instead add a way to select some sub-set of tests to run? I.e., let's break the set of tests into two classes: already existing linear register tests and and your new stuff. By default, both sets should be run.
--
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: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 12 Apr 2018 22:44:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
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 <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 23 Apr 2018 19:17:18 +0000
Gerrit-HasComments: Yes