Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11516 )
Change subject: tserver: log a message when a scanner is not found ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc@2015 PS1, Line 2015: Status verbose = s.CloneAndAppend(Substitute("call sequence id=$0, remote=$1", : req->call_seq_id(), : rpc_context->requestor_string())); Why not to put this information into the returned status as is? http://gerrit.cloudera.org:8080/#/c/11516/1/src/kudu/tserver/tablet_service.cc@2038 PS1, Line 2038: if (req->call_seq_id() != scanner->call_seq_id()) { : *error_code = TabletServerErrorPB::INVALID_SCAN_CALL_SEQ_ID; : return Status::InvalidArgument("Invalid call sequence ID in scan request"); : } Some unrelated to this particular change, but maybe relevant stuff when chasing the not-found-scanner issue. While you are here, do you think it's worth making some other changes (maybe, in a separate changelist): 1) Moving this verification before instantiating ScopedUnregisterScanner. The reason is that current code inadvertently (I think) closes the scanner if a client sends in wrong scan call sequence id with its request for a valid scanner identifier. 2) Add a trace or VLOG() message for the case of the scenario in L2026 of the original code, since it makes the scanner to be automatically closed after the return. 3) Fill in the 'has_more_results output' parameter in case of situation as in L2007 in the original code. -- To view, visit http://gerrit.cloudera.org:8080/11516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If4e9c69160605d4a839ac2d28def67f4d3402b90 Gerrit-Change-Number: 11516 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 26 Sep 2018 17:58:21 +0000 Gerrit-HasComments: Yes
