Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13536 )
Change subject: KUDU-2809 (4/6): skip unobservable rows when iterating a DRS ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.h File src/kudu/tablet/delta_store.h: http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.h@133 PS2, Line 133: // Comparator that establishes a total ordering amongst Deltas. Obvious detail worth mentioning: "a total ordering amongst Deltas for the same row" http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc File src/kudu/tablet/delta_store.cc: http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@114 PS2, Line 114: // delta type | oldest | newest Thanks for including this helpful block comment. http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@156 PS2, Line 156: existing->newest Why include newest? When could existing->oldest not be less than existing->newest? http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@158 PS2, Line 158: existing->oldest same http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@278 PS2, Line 278: Traits::kType, nit: Seems worth writing a constructor to guarantee all fields get set so we don't have to worry about it at the call sites. http://gerrit.cloudera.org:8080/#/c/13536/2/src/kudu/tablet/delta_store.cc@281 PS2, Line 281: decoder.get_type() }; Should be: reinterpret_cast<size_t>(this) or similar; also maybe consider using uintptr_t -- To view, visit http://gerrit.cloudera.org:8080/13536 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2b616c4e8b99dbc7063b940bcb35352f87ab0226 Gerrit-Change-Number: 13536 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Fri, 07 Jun 2019 07:02:05 +0000 Gerrit-HasComments: Yes
