David Ribeiro Alves has posted comments on this change. Change subject: [java] Reuse snapshot scan timestamp across tablets ......................................................................
Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/5188/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java: PS6, Line 650: Server-assigned timestamp for the scan operation. It's used when : * the scan operates in READ_AT_SNAPSHOT mode and the timestamp is not : * specified explicitly. The field is set with the snapshot timestamp sent : * in the response from the very first tablet server contacted while : * fetching data from corresponding tablets. If the tablet server does not : * send the snapshot timestamp in its response, this field is assigned : * a special value AsyncKuduClient.NO_TIMESTAMP. > If so, then it contradicts with the comment for $REPO_ROOT/src/kudu/tserver maybe I didn't explain myself correctly. It was not about adding a new test. It was about adding a checkState() or whatever for the following: If the server sent a timestamp on the response (i.e. if the 'timestamp' field on the response is set or is != from NO_TIMESTAMP or whatever the java client calls it) then: If a timestamp was set on the scanner before, they should match so we should add a check to make sure they do. If a timestamp was not set on the scanner before, set it. also nit: could you rename serverScanTimestamp to responseScanTimestamp? -- To view, visit http://gerrit.cloudera.org:8080/5188 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7207672f7b0cf1307bfa861bda3291b278618016 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
