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 2017/12/22 01:13:06 UTC
[kudu-CR] KUDU-2236 pt 2: fix flake in TestKuduClient
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8908
Change subject: KUDU-2236 pt 2: fix flake in TestKuduClient
......................................................................
KUDU-2236 pt 2: fix flake in TestKuduClient
The original patch d78b2727d1246069b2006ee652c3ff9a2005601c was too
loose in what exceptions it would allow.
This patch addresses this by being more selective in the allowed errors:
- "RecoverableException: connection [closed/disconnected]": these were
expected even before KUDU-2236 but the condition was too loose in pt 1
- "disconnected from peer": caused by explicit disconnections of the
peer. Logging from is disconnections are expected.
Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
1 file changed, 6 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/8908/1
--
To view, visit http://gerrit.cloudera.org:8080/8908
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
Gerrit-Change-Number: 8908
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] KUDU-2236 pt 2: fix flake in TestKuduClient
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8908 )
Change subject: KUDU-2236 pt 2: fix flake in TestKuduClient
......................................................................
KUDU-2236 pt 2: fix flake in TestKuduClient
The original patch d78b2727d1246069b2006ee652c3ff9a2005601c was too
loose in what exceptions it would allow. A more elegant way to uphold
the original intention of the test (i.e. to avoid log spew caused by
an unexpected disconnection) is to verify that there are no messages
complaining about unexpectedly-disconnected connections.
Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
Reviewed-on: http://gerrit.cloudera.org:8080/8908
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
1 file changed, 3 insertions(+), 5 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/8908
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
Gerrit-Change-Number: 8908
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: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2236 pt 2: fix flake in TestKuduClient
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Dan Burkert, Kudu Jenkins, Hao Hao, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8908
to look at the new patch set (#2).
Change subject: KUDU-2236 pt 2: fix flake in TestKuduClient
......................................................................
KUDU-2236 pt 2: fix flake in TestKuduClient
The original patch d78b2727d1246069b2006ee652c3ff9a2005601c was too
loose in what exceptions it would allow. A more elegant way to uphold
the original intention of the test (i.e. to avoid log spew caused by
an unexpected disconnection) is to verify that there are no messages
complaining about unexpectedly-disconnected connections.
Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
1 file changed, 3 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/8908/2
--
To view, visit http://gerrit.cloudera.org:8080/8908
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
Gerrit-Change-Number: 8908
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: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2236 pt 2: fix flake in TestKuduClient
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8908 )
Change subject: KUDU-2236 pt 2: fix flake in TestKuduClient
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8908/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/8908/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@845
PS1, Line 845: exception_text.contains("RecoverableException: connection closed") ||
: exception_text.contains("RecoverableException: connection disconnected") ||
: exception_text.contains("disconnected from peer"));
> As I understand, all we want to check is that we don't see anything about u
Yeah, that sounds reasonable.
--
To view, visit http://gerrit.cloudera.org:8080/8908
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
Gerrit-Change-Number: 8908
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 22 Dec 2017 22:01:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2236 pt 2: fix flake in TestKuduClient
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8908 )
Change subject: KUDU-2236 pt 2: fix flake in TestKuduClient
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8908/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/8908/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@845
PS1, Line 845: exception_text.contains("RecoverableException: connection closed") ||
Did you get a chance to loop the test before and after this patch? I recall when you loop the last patch it was 100/100 pass rate. I thought that implies all possible exceptions are already listed in previous patch?
--
To view, visit http://gerrit.cloudera.org:8080/8908
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
Gerrit-Change-Number: 8908
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 22 Dec 2017 01:52:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2236 pt 2: fix flake in TestKuduClient
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8908 )
Change subject: KUDU-2236 pt 2: fix flake in TestKuduClient
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8908/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/8908/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@845
PS1, Line 845: exception_text.contains("RecoverableException: connection closed") ||
: exception_text.contains("RecoverableException: connection disconnected") ||
: exception_text.contains("disconnected from peer"));
As I understand, all we want to check is that we don't see anything about unexpected disconnection, right? If so, then maybe, just check that the output does not contain 'lost connection to peer' string?
What do you think?
--
To view, visit http://gerrit.cloudera.org:8080/8908
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
Gerrit-Change-Number: 8908
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 22 Dec 2017 19:08:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2236 pt 2: fix flake in TestKuduClient
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8908 )
Change subject: KUDU-2236 pt 2: fix flake in TestKuduClient
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8908
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
Gerrit-Change-Number: 8908
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: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Sat, 23 Dec 2017 01:59:45 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2236 pt 2: fix flake in TestKuduClient
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8908 )
Change subject: KUDU-2236 pt 2: fix flake in TestKuduClient
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8908/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/8908/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@845
PS1, Line 845: exception_text.contains("RecoverableException: connection closed") ||
> Did you get a chance to loop the test before and after this patch? I recall
Yeah, it's because the previous patch was not very strict with what it would allow. E.g. I tried doing something silly:
assertTrue(cla.getAppendedText().contains("connection disconnected") // i.e. no exceptions
This actually passed, because "connection disconnected" gets logged a lot, vs "RecoverableException: connection disconnected", which is much more uncommon. This patch still passes 1000/1000, but with stricter conditions.
--
To view, visit http://gerrit.cloudera.org:8080/8908
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
Gerrit-Change-Number: 8908
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 22 Dec 2017 01:58:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2236 pt 2: fix flake in TestKuduClient
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8908 )
Change subject: KUDU-2236 pt 2: fix flake in TestKuduClient
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8908/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/8908/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@845
PS1, Line 845: exception_text.contains("RecoverableException: connection closed") ||
> Yeah, it's because the previous patch was not very strict with what it woul
Cool, thanks for the explanation.
--
To view, visit http://gerrit.cloudera.org:8080/8908
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7aafdc0eca00e743048ecc099dcb3241ce7ac8ad
Gerrit-Change-Number: 8908
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 22 Dec 2017 05:09:52 +0000
Gerrit-HasComments: Yes