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 2019/01/26 01:17:00 UTC
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12276
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
[java] deflake RYW tests in TestKuduClient
RYW tests in TestKuduClient is flaky which fails with AssertionError
when verifying the chosen snapshot timestamp returned from the server
is larger than the previous propagated timestamp before the scan.
It turns out to be a test only issue, which due to incorrect propagated
timestamp is being used.
Without the fix, 192/1000 runs of TestKuduClient failed with this error.
With the fix, 0/1000 runs of TestKuduClient failed.
Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
2 files changed, 8 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/12276/1
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/12276 )
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12276/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/12276/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1084
PS1, Line 1084: // never go down from what previously observed, to ensure subsequent
: // reads will not "go back in time" regarding writes that other
: // clients have done.
: for (int k = 0; k < 3; k++) {
: AsyncKuduScanner scanner = asyncKuduClient.newScannerBuilder(table)
: .readMode(AsyncKuduScanner.ReadMode.READ_YO
> I don't see anything in this block of code that would cause the client's la
Actually after more thoughts, this structure of the test is actually not right. The same client is being used concurrently (that is why there is concurrent update to LastPropagatedTimestamp), while it aims to test multiple clients. I will correct it.
http://gerrit.cloudera.org:8080/#/c/12276/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1090
PS1, Line 1090: .replicaSelection(replicaSelection)
: .build();
> Should this test preTs instead of a new call to getLastPropagatedTimestamp?
Done
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 27 Jan 2019 20:54:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12276 )
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
[java] deflake RYW tests in TestKuduClient
RYW tests in TestKuduClient is flaky which fails with AssertionError
when verifying the chosen snapshot timestamp returned from the server
is larger than the previous propagated timestamp before the scan.
It turns out to be a test only issue, which due to incorrect structure
of the test. It aims to test READ_YOUR_WRITE mode for multiple clients,
while the same client was being used. The distinction is important,
because we want to ensure a single client can still preserve
read-your-writes and read-your-reads session guarantees even there are
concurrent reads/writes performed by other clients in READ_YOUR_WRITE
scan mode.
Without the fix, 192/1000 runs of TestKuduClient failed with this error.
With the fix, 0/1000 runs of TestKuduClient failed.
Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Reviewed-on: http://gerrit.cloudera.org:8080/12276
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
2 files changed, 15 insertions(+), 8 deletions(-)
Approvals:
Kudu Jenkins: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/12276 )
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12276/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12276/3//COMMIT_MSG@13
PS3, Line 13: It turns out to be a test only issue, which due to incorrect structure
> OK, so if I'm understanding this correctly, the intent of the test was alwa
The distinction is important because we want to ensure a single client can preserve read-your-writes and read-your-reads session guarantees even there is concurrent reads/writes performed by other clients.
http://gerrit.cloudera.org:8080/#/c/12276/3//COMMIT_MSG@15
PS3, Line 15: wa
> Nit: 'was'
Done
http://gerrit.cloudera.org:8080/#/c/12276/3//COMMIT_MSG@18
PS3, Line 18: concurrent reads/writes performed by other clients
> I presume this is still true with the new version of the test? Do you know
Right, it passed with the new version.
Sorry that I added the line which closed the client at the wrong place (https://gerrit.cloudera.org/#/c/12276/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1108) after I looped the second version...
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 28 Jan 2019 20:08:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12276 )
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12276/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12276/3//COMMIT_MSG@13
PS3, Line 13: It turns out to be a test only issue, which due to incorrect structure
OK, so if I'm understanding this correctly, the intent of the test was always to test multiple clients, and it was a mistake for the test to share a single client the way it did? Could you explain why the distinction is important, based on RYW semantics?
http://gerrit.cloudera.org:8080/#/c/12276/3//COMMIT_MSG@15
PS3, Line 15: is
Nit: 'was'
http://gerrit.cloudera.org:8080/#/c/12276/3//COMMIT_MSG@18
PS3, Line 18: With the fix, 0/1000 runs of TestKuduClient failed.
I presume this is still true with the new version of the test? Do you know why the test also passed 1000 times in PS2?
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 28 Jan 2019 19:41:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12276 )
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 28 Jan 2019 23:38:57 +0000
Gerrit-HasComments: No
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12276
to look at the new patch set (#3).
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
[java] deflake RYW tests in TestKuduClient
RYW tests in TestKuduClient is flaky which fails with AssertionError
when verifying the chosen snapshot timestamp returned from the server
is larger than the previous propagated timestamp before the scan.
It turns out to be a test only issue, which due to incorrect structure
of the test. It aims to test READ_YOUR_WRITE mode for multiple clients,
while the same client is being used.
Without the fix, 192/1000 runs of TestKuduClient failed with this error.
With the fix, 0/1000 runs of TestKuduClient failed.
Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
2 files changed, 15 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/12276/3
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/12276 )
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
Patch Set 3: Verified+1
Unrelated flaky test.
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 28 Jan 2019 17:56:21 +0000
Gerrit-HasComments: No
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12276
to look at the new patch set (#4).
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
[java] deflake RYW tests in TestKuduClient
RYW tests in TestKuduClient is flaky which fails with AssertionError
when verifying the chosen snapshot timestamp returned from the server
is larger than the previous propagated timestamp before the scan.
It turns out to be a test only issue, which due to incorrect structure
of the test. It aims to test READ_YOUR_WRITE mode for multiple clients,
while the same client was being used. The distinction is important,
because we want to ensure a single client can still preserve
read-your-writes and read-your-reads session guarantees even there are
concurrent reads/writes performed by other clients in READ_YOUR_WRITE
scan mode.
Without the fix, 192/1000 runs of TestKuduClient failed with this error.
With the fix, 0/1000 runs of TestKuduClient failed.
Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
2 files changed, 15 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/12276/4
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12276
to look at the new patch set (#2).
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
[java] deflake RYW tests in TestKuduClient
RYW tests in TestKuduClient is flaky which fails with AssertionError
when verifying the chosen snapshot timestamp returned from the server
is larger than the previous propagated timestamp before the scan.
It turns out to be a test only issue, which due to incorrect structure
of the test. It aims to test READ_YOUR_WRITE mode for multiple clients,
while the same client is being used.
Without the fix, 192/1000 runs of TestKuduClient failed with this error.
With the fix, 0/1000 runs of TestKuduClient failed.
Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
2 files changed, 15 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/12276/2
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12276 )
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12276/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/12276/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1084
PS1, Line 1084:
: AsyncKuduScanner scanner = asyncClient.newScannerBuilder(table)
: .readMode(AsyncKuduScanner.ReadMode.READ_YOUR_WRITES)
: .replicaSelection(replicaSelection)
: .build();
: KuduScanner syncScanner = new KuduScanner(scanner);
I don't see anything in this block of code that would cause the client's lastPropagatedTimestamp value to change. How does moving the initialization of preTs address the flakiness?
http://gerrit.cloudera.org:8080/#/c/12276/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1090
PS1, Line 1090: assertNotEquals(AsyncKuduClient.NO_TIMESTAMP,
: asyncClient.getLastPropagatedTimestamp());
Should this test preTs instead of a new call to getLastPropagatedTimestamp?
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 27 Jan 2019 18:38:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] deflake RYW tests in TestKuduClient
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.
Change subject: [java] deflake RYW tests in TestKuduClient
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/12276
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I951abb9197f7e6b6a4c70cdf89948206840ddeda
Gerrit-Change-Number: 12276
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)