Alexey Serbin has posted comments on this change. Change subject: KUDU-1034 client does not failover due to timeout ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/6924/1/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: Line 420: // Test that a client fails over when the leader times out. > This is the original location for the test as in the patch attached to KUDU After some consideration I think it's also a good place for the test since it verifies the consistency overall client-->tserver communications when the client retries the operation against other tablet replica. PS1, Line 450: for (int i = 0; i < 1000; i++) { : if (workload.rows_inserted() >= rows_target) break; : SleepFor(MonoDelta::FromMilliseconds(10)); : } > It seems this test does not provide the necessary coverage: the Workload do I take it back: after some consideration I think that in the scope of KUDU-1034 this is the exact test what is needed. The client should fail in case if RPC times out, and it's the responsibility of the upper-level code to retry an operation in that case. Otherwise, if client would retry on RPC timeout, that could lead to retrying indefinitely. -- To view, visit http://gerrit.cloudera.org:8080/6924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icfcece485e4053d921ffdc865612b3e7b9a992a3 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
