Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/3654/3/java/kudu-client/src/main/java/org/kududb/client/GetMasterRegistrationReceived.java
File 
java/kudu-client/src/main/java/org/kududb/client/GetMasterRegistrationReceived.java:

PS3, Line 109: and
             :         // there's as many of them as responses received.
> This much is obvious from the code itself, but what's not obvious is the si
Good idea.


http://gerrit.cloudera.org:8080/#/c/3654/3/java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java
File java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java:

Line 358:     waitForAllTabletServers();
> I've been working on a change that'll allow ListTabletServers to be called 
I had a stale server-side compilation, with your new code it works. Sorry!


http://gerrit.cloudera.org:8080/#/c/3654/3/java/kudu-client/src/test/java/org/kududb/client/TestGetMasterRegistrationReceived.java
File 
java/kudu-client/src/test/java/org/kududb/client/TestGetMasterRegistrationReceived.java:

Line 91:     // Permutation of the previous
> Nit: terminate with a period. Below too.
Done


Line 162:     GetMasterRegistrationReceived grrm = new 
GetMasterRegistrationReceived(MASTERS, d);
> Nice variable name.
Thanks! :D


Line 177:       d.join(1000); // Don't care about the response.
> Is there any significance to the timeout value here? Could we call join() w
I forgot to remove it after adding a timeout on the test itself.


Line 188:   // Helper method that determines if the callback or errback should 
be called.
> This one and makeGMRR() can be static.
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-HasComments: Yes

Reply via email to