David Ribeiro Alves has posted comments on this change. Change subject: WIP: KUDU-1189 if not set, use timestamp from first server ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/5143/1/src/kudu/client/scanner-internal.cc File src/kudu/client/scanner-internal.cc: PS1, Line 408: configuration_.snapshot_timestamp() != ScanConfiguration::kNoTimestam maybe add a has_snapshot_timestamp() to scan_configuration? PS1, Line 408: configuration_.snapshot_timestamp() != ScanConfiguration::kNoTimestamp in general, does this cover the case where the user didn't specify a timestamp but the first server that was hit chose one for the scan? That _is_ the core thing we're trying to implement/test here, IIRC in the case where the user does specify a timestamp we already set it in all the scans. PS1, Line 409: last_response_.has_snap_timestamp() when would this not be set? Not sure what to do there: maybe crash? maybe return an error? in any case this is something that has bene in the server for quite a while and should be compatible with the foreseeable past client/server versions. -- To view, visit http://gerrit.cloudera.org:8080/5143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ac708e38b8a80834f7d54eca294517cbfb06ec6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
