Adar Dembo has posted comments on this change.
Change subject: [java-client] Re-enable multi-master tests
Patch Set 3:
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
significance. Could you modify the comment to explain why this matters?
Line 358: waitForAllTabletServers();
I've been working on a change that'll allow ListTabletServers to be called on
follower masters, as that reflects the new world order, where all masters
receive heartbeats in order to keep their TS caches warm.
With that change in place, I don't think waitForAllTabletServers() can be used
as a proxy for "wait for a new master to be elected and for tservers to report
to it". getTablesList() comes close (because it can only succeed after a new
leader master is elected) but that's still not the same thing.
Why do you actually need to wait? AFAICT killMasterLeader() is only used in
TestMasterFailover.testKillLeader(), and the operations that follow the call
should all succeed even without waiting, because the client itself is supposed
to wait. The only exception is the createTable(), but that will be fixed with
Line 91: // Permutation of the previous
Nit: terminate with a period. Below too.
Line 162: GetMasterRegistrationReceived grrm = new
Nice variable name.
Line 177: d.join(1000); // Don't care about the response.
Is there any significance to the timeout value here? Could we call join()
without a timeout?
Line 188: // Helper method that determines if the callback or errback should
This one and makeGMRR() can be static.
To view, visit http://gerrit.cloudera.org:8080/3654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>