Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9216 )
Change subject: tablet: don't store row count in delta tracker ...................................................................... Patch Set 5: (7 comments) http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h File src/kudu/tablet/diskrowset.h: http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h@366 PS5, Line 366: Status CountRows(rowid_t *count) const override; > can we mark this final so that calls from within this class can be inlined Done http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h@474 PS5, Line 474: // Number of rows in the rowset. > can you add a comment here that this might be unset? Done http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h@475 PS5, Line 475: mutable std::atomic<rowid_t> num_rows_; > let's just use int64_t here, rowid_t is kinda old and gross Done http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc File src/kudu/tablet/diskrowset.cc: http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@522 PS5, Line 522: num_rows_(0), > let's use -1 as a signifier of unknown here Done http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@672 PS5, Line 672: rowid_t num_rows; : RETURN_NOT_OK(CountRows(&num_rows)); > this can be inside an #ifndef NDEBUG right? Done http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@700 PS5, Line 700: rowid_t num_rows; > same Done http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@725 PS5, Line 725: { > I don't think the extra nesting is worth making the critical section one st Done -- To view, visit http://gerrit.cloudera.org:8080/9216 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d Gerrit-Change-Number: 9216 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Tue, 06 Feb 2018 16:13:44 +0000 Gerrit-HasComments: Yes
