David Ribeiro Alves has posted comments on this change. Change subject: [java] Reuse snapshot scan timestamp across tablets ......................................................................
Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/5188/5/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: PS5, Line 193: /** : * 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. Initially, this field is assigned a special value : * which means 'unset'. 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. : */ : private long serverScanTimestamp = AsyncKuduClient.NO_TIMESTAMP; any specific reason you don't want to reuse "htTimestamp"? PS5, Line 369: // To be used in tests only. : protected long getServerScanTimestamp() { : return this.serverScanTimestamp; : } I think this is weird. What is the difference between this getter and the one above? They're both snapshot timestamps... http://gerrit.cloudera.org:8080/#/c/5188/5/java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java: PS5, Line 209: int rowCount = 0; : while (syncScanner.hasMoreRows()) { : rowCount += syncScanner.nextRows().getNumRows(); : final long ts = scanner.getServerScanTimestamp(); : assertNotEquals(AsyncKuduClient.NO_TIMESTAMP, ts); : if (isTimestampSet) { : assertEquals(tsRef, ts); : } else { : isTimestampSet = true; : tsRef = ts; : } : } If we merge htTimestamp with serverScanTimestamp I think this test could go: - assert htTimestamp is not set - assert scanner has more rows - get more rows - assert htTimestamp is set - scan the rest of the rows - assert htTimestamp is the same -- 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: 5 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
