Adar Dembo has posted comments on this change. Change subject: [client] retry operation in case of ServiceUnavailable ......................................................................
Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/5964/6/src/kudu/client/client-internal.cc File src/kudu/client/client-internal.cc: PS6, Line 241: if (s.ok() && resp->has_error()) { : if (resp->error().code() == MasterErrorPB::NOT_THE_LEADER || : resp->error().code() == MasterErrorPB::CATALOG_MANAGER_NOT_INITIALIZED) { : if (client->IsMultiMaster()) { : KLOG_EVERY_N_SECS(INFO, 1) << "Determining the new leader Master and retrying..."; : WARN_NOT_OK(ConnectToCluster(client, deadline), : "Unable to determine the new leader Master"); : } : continue; : } else { : return StatusFromPB(resp->error().status()); : } : } FWIW, I don't believe this case can be reached right now. I don't know if it was ever reachable, to be honest. AFAICT, the catalog manager responds with ServiceUnavailable or IllegalState if it also sets one of these errors. But, it's possible that this wasn't the case in the past; I don't know for sure. http://gerrit.cloudera.org:8080/#/c/5964/6/src/kudu/integration-tests/create-table-stress-test.cc File src/kudu/integration-tests/create-table-stress-test.cc: Line 337: SleepFor(MonoDelta::FromMilliseconds(1 + rand() % 10)); Why this change? Line 351: if (s.IsTimedOut()) { Could we set the default admin operation timeout of the client real high and then avoid this case altogether? -- To view, visit http://gerrit.cloudera.org:8080/5964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iae169ce1efe3950dfcb769594a1944d05e2ad8e7 Gerrit-PatchSet: 6 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: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
