You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2020/07/20 22:25:06 UTC
[kudu-CR] KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Hello Alexey Serbin, Grant Henke,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/16221
to review the following change.
Change subject: KUDU-3170: deflake testSlowWriterDoesntBlockQueues
......................................................................
KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Previously the test would block the writer thread for 1s, expect a
timeout to happen within 1s, and check that the slow writer still
allowed for the outbound queue to be inserted to.
This led to some flakiness, as the first write may have sometimes
completed within 1s and allowed the second request to leave the outbound
queue and begin writing. This patch addresses this by extending the time
with which we block writes to extend past the execution deadline.
Without this patch, we saw frequent failures in ARM runs of the test,
and I saw 4/400 failures in dist-test. With the patch, the test passed
in dist-test 1600/1600 times.
Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
---
M java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/16221/1
--
To view, visit http://gerrit.cloudera.org:8080/16221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
Gerrit-Change-Number: 16221
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
[kudu-CR] KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16221 )
Change subject: KUDU-3170: deflake testSlowWriterDoesntBlockQueues
......................................................................
Patch Set 2: Verified+1
Interesting but unrelated failure of TokenTest.TestKeyValidity
Error Message
/home/jenkins-slave/workspace/kudu-master/1/src/kudu/security/token-test.cc:843
Expected: VerificationResult::EXPIRED_TOKEN
Which is: 4-byte object <03-00 00-00>
To be equal to: result
Which is: 4-byte object <00-00 00-00>
validation result 'valid': authn token expires at 1595300777, now 1595300777.7028718
--
To view, visit http://gerrit.cloudera.org:8080/16221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
Gerrit-Change-Number: 16221
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 21 Jul 2020 03:17:09 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16221 )
Change subject: KUDU-3170: deflake testSlowWriterDoesntBlockQueues
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/16221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
Gerrit-Change-Number: 16221
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 21 Jul 2020 01:23:05 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16221 )
Change subject: KUDU-3170: deflake testSlowWriterDoesntBlockQueues
......................................................................
KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Previously the test would block the writer thread for 1s, expect a
timeout to happen within 1s, and check that the slow writer still
allowed for the outbound queue to be inserted to.
This led to some flakiness, as the first write may have sometimes
completed within 1s and allowed the second request to leave the outbound
queue and begin writing. This patch addresses this by extending the time
with which we block writes to extend past the execution deadline.
Without this patch, we saw frequent failures in ARM runs of the test,
and I saw 4/400 failures in dist-test. With the patch, the test passed
in dist-test 1600/1600 times.
Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
Reviewed-on: http://gerrit.cloudera.org:8080/16221
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Andrew Wong <aw...@cloudera.com>
---
M java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java
1 file changed, 4 insertions(+), 3 deletions(-)
Approvals:
Grant Henke: Looks good to me, approved
Andrew Wong: Verified
--
To view, visit http://gerrit.cloudera.org:8080/16221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
Gerrit-Change-Number: 16221
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16221 )
Change subject: KUDU-3170: deflake testSlowWriterDoesntBlockQueues
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16221/1/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java
File java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java:
http://gerrit.cloudera.org:8080/#/c/16221/1/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java@288
PS1, Line 288: The MessageWriter took the first message from the outbound queue and
: // went to sleep f
> nit: could you add a small blurb explaining why the first message should ha
Done
--
To view, visit http://gerrit.cloudera.org:8080/16221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
Gerrit-Change-Number: 16221
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 21 Jul 2020 02:43:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16221 )
Change subject: KUDU-3170: deflake testSlowWriterDoesntBlockQueues
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
Gerrit-Change-Number: 16221
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 21 Jul 2020 03:16:10 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: KUDU-3170: deflake testSlowWriterDoesntBlockQueues
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/16221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
Gerrit-Change-Number: 16221
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16221 )
Change subject: KUDU-3170: deflake testSlowWriterDoesntBlockQueues
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16221/1/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java
File java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java:
http://gerrit.cloudera.org:8080/#/c/16221/1/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java@288
PS1, Line 288: We should see a single message in the outbound queue. The other one is
: // blocked writing
nit: could you add a small blurb explaining why the first message should have been removed from the outbound queue already, but the second is still there? Something like:
'The MessageWriter took the first message from the outbound queue and went sleeping for 2 * TIMEOUT_MS, but the second message should still be in the queue after TIMEOUT_MS.'
--
To view, visit http://gerrit.cloudera.org:8080/16221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
Gerrit-Change-Number: 16221
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 21 Jul 2020 00:18:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16221
to look at the new patch set (#2).
Change subject: KUDU-3170: deflake testSlowWriterDoesntBlockQueues
......................................................................
KUDU-3170: deflake testSlowWriterDoesntBlockQueues
Previously the test would block the writer thread for 1s, expect a
timeout to happen within 1s, and check that the slow writer still
allowed for the outbound queue to be inserted to.
This led to some flakiness, as the first write may have sometimes
completed within 1s and allowed the second request to leave the outbound
queue and begin writing. This patch addresses this by extending the time
with which we block writes to extend past the execution deadline.
Without this patch, we saw frequent failures in ARM runs of the test,
and I saw 4/400 failures in dist-test. With the patch, the test passed
in dist-test 1600/1600 times.
Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
---
M java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java
1 file changed, 4 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/16221/2
--
To view, visit http://gerrit.cloudera.org:8080/16221
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b322782630b3ef935fb0264b6195d2ed72cc58c
Gerrit-Change-Number: 16221
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)