Adar Dembo 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 cluster and waits for the tservers to finish starting. Seems like it was copied from the previous test which explicitly restarts the tservers. 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? 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 to skip test-only stuff and focus on code with a production impact. -- 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 16:50:13 +0000 Gerrit-HasComments: Yes
