Mike Percy has posted comments on this change. Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover ......................................................................
Patch Set 5: (9 comments) looks good, just a couple minor things now http://gerrit.cloudera.org:8080/#/c/7456/4/java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java: Line 39: @Override > Ok, I redid it with intellij's settings. Does it seem correct now? lgtm now, thanks! http://gerrit.cloudera.org:8080/#/c/7456/5/java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestMultipleLeaderFailover.java: PS5, Line 35: int it's typical to use a long for millisecond time values in Java PS5, Line 64: OUTER_LOOP still named OUTER_LOOP, see comments on rev 4 PS5, Line 71: 30000 Better to not use magic numbers. For consistency with the rest of the client test codebase (even though it's a timeout, not a sleep), it seems best to use DEFAULT_SLEEP for the timeout value here and elsewhere. PS5, Line 89: yet no need to say "yet" here, RYW will always be optional PS5, Line 90: 30000 DEFAULT_SLEEP PS5, Line 93: 30000 use DEFAULT_SLEEP here and elsewhere http://gerrit.cloudera.org:8080/#/c/7456/5/java/kudu-client/src/test/java/org/apache/kudu/util/AssertHelpers.java File java/kudu-client/src/test/java/org/apache/kudu/util/AssertHelpers.java: PS5, Line 27: Note that any variables in BooleanExpression.get() will need to be final This is a standard Java thing and not required to be mentioned here Line 28: // Borrowed from Flume. No need to give credit for this. Flume is ASL 2.0 which doesn't require attribution, plus I originally wrote it and it's fine with me not to give attribution. -- To view, visit http://gerrit.cloudera.org:8080/7456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Edward Fancher <[email protected]> Gerrit-Reviewer: Edward Fancher <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
