Alexey Serbin has posted comments on this change.

Change subject: KUDU-2021 test for negotiation timeout on Master RPCs
......................................................................


Patch Set 8:

(4 comments)

> (4 comments)
 > 
 > Would you mind running 1000 iters on this test on dist-test just to
 > make sure it's not flaky?

I did that already several times and did that once more for the DEBUG build:
  http://dist-test.cloudera.org//job?job_id=aserbin.1495742228.16502

The failures you can see there are attributed only to the port bind errors 
(e.g. something like 'Address already in use (error 98)')

I think your idea to separate the set of multi-master tests into a separate 
test binary and running it with appropriate RESOURCE_LOCK is a good idea.

http://gerrit.cloudera.org:8080/#/c/6927/8/src/kudu/integration-tests/client_failover-itest.cc
File src/kudu/integration-tests/client_failover-itest.cc:

PS8, Line 539:  
> start the negotiation process
Done


PS8, Line 540: k cl
> the client's
Done


PS8, Line 544:  ne
> the negotiation process
Done


PS8, Line 553: FLAGS_rpc_negotiation_timeout_ms
> Is this reliable? Should it be the rpc timeout * 2 instead just to make sur
It's a good point.

I think it is reliable -- that's because there are some extra steps that takes 
some time at the server side, so the total is usually tens of ms more than the 
injected delay at least in DEBUG builds.  However, I think it's better to bump 
it a little as suggested to increase trust in this test and to be on the safe 
side :)

As for shortening the default RPC negotation timeout for the client: it's 
already so. Check the constructor of the 
ClientFailoverOnNegotiationTimeoutITest class -- it's set the re to just 1 
second.

I'm not sure whether we want to have that for the server-side components.  My 
thinking is the following: the only affected component here would be the leader 
master, and the Raft-related timeouts seem to be shorter anyway in that regard. 
 Let me know if you think I'm missing something.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib62126c9d8c6c65f447c5d03a0377eaff823393c
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to