David Ribeiro Alves has posted comments on this change. Change subject: tablet: check for row presence in rowset-wise order ......................................................................
Patch Set 10: (8 comments) http://gerrit.cloudera.org:8080/#/c/6483/10//COMMIT_MSG Commit Message: PS10, Line 50: This patch isn't meant to give big wins on its own, but the further : patches in the series ought to. nit: it would make this reviewer more at ease if I could take a peek at those numbers in a commit message somewhere. http://gerrit.cloudera.org:8080/#/c/6483/8/src/kudu/tablet/row_op.h File src/kudu/tablet/row_op.h: PS8, Line 85: Flag whether this op has already had 'present_in_rowset' filled in. > hrm... you mean down below for the comment on 'present_in_rowset' right? Wi oh, yeah. thanks http://gerrit.cloudera.org:8080/#/c/6483/10/src/kudu/tablet/row_op.h File src/kudu/tablet/row_op.h: PS10, Line 88: exists in no RowSet. nit: doesn't exist in any RowSet PS10, Line 93: to be alive in no RowSet. nit: not to be alive in any RowSet. http://gerrit.cloudera.org:8080/#/c/6483/10/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: PS10, Line 680: num_ops == 0 predict_false PS10, Line 698: // equivalent indexes. nit: no need to wrap http://gerrit.cloudera.org:8080/#/c/6483/10/src/kudu/tablet/tablet.h File src/kudu/tablet/tablet.h: PS10, Line 404: // invalid. nit: no need to wrap PS10, Line 407: // Validate 'op' as in 'ValidateOp()' above. If it is invalid, marks : // the op as failed and returns false. If valid, marks the op as validated : // and returns true. nit: two lines instead of three -- To view, visit http://gerrit.cloudera.org:8080/6483 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9a678bb0da130ce94aae3634c82ace2d6898ffa2 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
