Alexey Serbin has posted comments on this change. Change subject: KUDU-2027 retry scan RPC if negotiation times out ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/7037/2/src/kudu/client/scanner-internal.cc File src/kudu/client/scanner-internal.cc: PS2, Line 216: if (overall_deadline == deadline && overall_deadline <= MonoTime::Now()) { > so basically this is making sure that even in the case where the overall_de Right. The issue with the original code was that even if the timeout error happened prior to that overall deadline, this code would assume the whole timeout interval has passed. I was also thinking of employing the new functionality to distinguish between negotiation timeout and the RPC timeout itself, but in that case it would be the same criteria plus that RpcController::negotiation_failed() anyway, so I decided to not involve the new RpcController::negotiation_failed(). What is the best place to document this new behavior? PS2, Line 407: now + KuduClient::Data::ComputeExponentialBackoff(attempt)) - now > not sure I'm following this. isn't now + KuduClient::Data::ComputeExponenti That's all about parentheses (i.e. braces) :) This code does the following: sleep = Earliest(deadline, now + backoff) - now; http://gerrit.cloudera.org:8080/#/c/7037/2/src/kudu/integration-tests/client-negotiation-failover-itest.cc File src/kudu/integration-tests/client-negotiation-failover-itest.cc: PS2, Line 40: using kudu::client::ScanToStrings; > ordering Done PS2, Line 113: other > nit: not your fault but "another" The original version was written by me as well :) Done. -- To view, visit http://gerrit.cloudera.org:8080/7037 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05a3c38cc07c9b6edd1ae773d9850e92645f3b14 Gerrit-PatchSet: 2 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: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
