Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12592 )
Change subject: KUDU-2744: Expose RPC interface for diff scan ...................................................................... Patch Set 7: (5 comments) http://gerrit.cloudera.org:8080/#/c/12592/6/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/12592/6/src/kudu/tserver/tablet_server-test.cc@2364 PS6, Line 2364: auto iter = keys.begin(); > keys is just a vector so you could index into it with i. Done http://gerrit.cloudera.org:8080/#/c/12592/6/src/kudu/tserver/tablet_server-test.cc@2368 PS6, Line 2368: CHECK_OK(row.SetInt32(0, key)); > Doesn't this suggest that key, keys, i, new_val, and all the other int64_t Err, nice catch. So it turns out that the base class is kind of a mess and I didn't realize it. I'll fix the types in the base class so this is all consistent, and layer that patch before this one. http://gerrit.cloudera.org:8080/#/c/12592/6/src/kudu/tserver/tablet_server-test.cc@2455 PS6, Line 2455: // Send requests with timestamps past the ancient history mark. We should get : // reasonable failure messages. : TEST_F(TabletServerTest, TestDiffScanPastAHM) { > May want to rename the test (and update the comment) to account for the oth Done http://gerrit.cloudera.org:8080/#/c/12592/6/src/kudu/tserver/tablet_server-test.cc@2506 PS6, Line 2506: NO_FATALS(req_assert_invalid_argument( : "scan start timestamp is only supported in ORDERED order mode")); > TabletServerErrorPB::INVALID_SNAPSHOT seems like a strange case here. Good observation. I ended up going down a rat hole to fix this and seem to have made it back alive, albeit with another couple of patches I'll layer before this one. http://gerrit.cloudera.org:8080/#/c/12592/6/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/12592/6/src/kudu/tserver/tablet_service.cc@1973 PS6, Line 1973: const Timestamp& snap_end_timestamp) { > Note that this comparison allows for the two timestamps to match, which the Thanks, done -- To view, visit http://gerrit.cloudera.org:8080/12592 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9602b549dc499f92b2069b297e66f366e51fb312 Gerrit-Change-Number: 12592 Gerrit-PatchSet: 7 Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Greg Solovyev <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Comment-Date: Sat, 23 Mar 2019 04:45:44 +0000 Gerrit-HasComments: Yes
