Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11646 )
Change subject: client: add timeout duration and scan attempts to scanner errors ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/11646/3/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/11646/3/src/kudu/client/client-test.cc@5173 PS3, Line 5173: // As the tservers are still starting up, the scan will retry until it : // times out. The actual error should be embedded in the returned status. > The first sentence doesn't make sense; ClientTest::SetUp() starts the clust oops, thanks for the catch http://gerrit.cloudera.org:8080/#/c/11646/3/src/kudu/client/scanner-internal.cc File src/kudu/client/scanner-internal.cc: http://gerrit.cloudera.org:8080/#/c/11646/3/src/kudu/client/scanner-internal.cc@498 PS3, Line 498: s = HandleError(scan_status, deadline, blacklist, /* needs_reopen=*/ nullptr); : RETURN_NOT_OK(s); > Why this change? Spurious change -- I backed out an error handling change. Reverted. http://gerrit.cloudera.org:8080/#/c/11646/3/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/11646/3/src/kudu/tserver/tablet_service.cc@2021 PS3, Line 2021: if (PREDICT_FALSE(FLAGS_scanner_inject_service_unavailable_on_continue_scan)) { : return Status::ServiceUnavailable("Injecting service unavailable status on Scan due to " : "--scanner_inject_service_unavailable_on_continue_scan"); : } > Nit: would prefer if this came at the top of the function; makes it easier The intention behind this location was to only reject the service when it was possible to service but leave the rest of the logic as-is, i.e. if the scanner cannot be found ideally we would still return a scanner not found error. -- To view, visit http://gerrit.cloudera.org:8080/11646 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a8f731f029132d0894355098d5804840f09e7c2 Gerrit-Change-Number: 11646 Gerrit-PatchSet: 3 Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Wed, 17 Oct 2018 23:50:56 +0000 Gerrit-HasComments: Yes
