Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10926 )
Change subject: memrowset: support iteration with snap_to_exclude ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/10926/6/src/kudu/tablet/memrowset-test.cc File src/kudu/tablet/memrowset-test.cc: http://gerrit.cloudera.org:8080/#/c/10926/6/src/kudu/tablet/memrowset-test.cc@586 PS6, Line 586: // @1 > Nit: Zero base these @# comments so it lines up with the snaps indices belo The timestamp output isn't zero-based though: @1: row (string key="row", uint32 val=0) mutations=[@2(SET val=1), @3(DELETE), @4(REINSERT val=2)] http://gerrit.cloudera.org:8080/#/c/10926/6/src/kudu/tablet/memrowset-test.cc@620 PS6, Line 620: // Captures zero rows; a snapshot range [x, x) does not include anything. > Nit: What happens if exclude is beyond include? That's illegal, though there's nothing enforcing that today. I'd add enforcement but it's tricky to do (because MvccSnapshot isn't just one timestamp, it's actually one "lower bound", one "upper bound", and, optionally, several points in between. -- To view, visit http://gerrit.cloudera.org:8080/10926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9be2ab5af4d5223889e4545ae1db7cc0275480ce Gerrit-Change-Number: 10926 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Sun, 22 Jul 2018 23:42:43 +0000 Gerrit-HasComments: Yes
