Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8951 )
Change subject: KUDU-2251: rowset size can overflow int in RowSetInfo ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/8951/4/src/kudu/integration-tests/heavy-update-compaction-test.cc File src/kudu/integration-tests/heavy-update-compaction-test.cc: PS4: We talked about this a little bit in person: I think it'd be good to produce a second, smaller test that's targeted for this particular regression. Hopefully such a test could work without actually writing 2 GB to disk, so it could really be a simple unit test. The problem is that the generality of this test makes it hard to actually ensure that the bug doesn't regress. http://gerrit.cloudera.org:8080/#/c/8951/4/src/kudu/integration-tests/heavy-update-compaction-test.cc@92 PS4, Line 92: gscoped_ptr<KuduTableCreator> table_creator(client_->NewTableCreator()); unique_ptr in new code. http://gerrit.cloudera.org:8080/#/c/8951/4/src/kudu/integration-tests/heavy-update-compaction-test.cc@116 PS4, Line 116: void MakeRow(int64_t key, KuduPartialRow* row); For simplicity's sake, perhaps inline the definition of MakeRow here? http://gerrit.cloudera.org:8080/#/c/8951/4/src/kudu/integration-tests/heavy-update-compaction-test.cc@155 PS4, Line 155: ASSERT_NO_FATAL_FAILURE(CreateTable()); Nit: use the shorter NO_FATALS macro. -- To view, visit http://gerrit.cloudera.org:8080/8951 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74975cdab605b51617d93d1ae98ef72ce87e35cb Gerrit-Change-Number: 8951 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Fri, 05 Jan 2018 21:55:13 +0000 Gerrit-HasComments: Yes
