KeDeng has posted comments on this change. ( http://gerrit.cloudera.org:8080/19670 )
Change subject: [GC] gc ancient, fully deleted rowsets without live row count ...................................................................... Patch Set 6: (6 comments) Thnaks for your reviews. http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc File src/kudu/tablet/diskrowset-test.cc: http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc@771 PS5, Line 771: TestGCDRSWithoutLiveRowCount) { > I think it's necessary to test that when the rows of this DRS have been ful Done http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc@773 PS5, Line 773: WriteTestRowSet(); > The function declaration is Well. I think there may be a misunderstanding here. The real work function is 'DoWriteTestRowSet', in its implementation, if the value of n_ rows is 0, which will be set to 10000. http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc@780 PS5, Line 780: CHECK_OK(rs->CountLiveRows(&rs_live_rows)); > nit: check the accurate value of 'rs_live_rows' Done http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc@790 PS5, Line 790: // The live rows count is 0 if the DRS has been fully deleted without DMS exist. > nit: Check 'rs_live_rows' is not change after some update operations. Done http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc@801 PS5, Line 801: WriteTestRowSet(); > Also to check that 'rs_live_rows_without_lrc' is larger than some value? Considering that the data in DMS may not be recorded, I added 'ASSERT'_ GE 'judgment. http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc@802 PS5, Line 802: kRowSet> rs; > I'm curious what's the purpose of this compact in this test? Removed. -- To view, visit http://gerrit.cloudera.org:8080/19670 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacdff107b8b07cbd56f47f296a93f4bcfbf56b41 Gerrit-Change-Number: 19670 Gerrit-PatchSet: 6 Gerrit-Owner: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org> Gerrit-Comment-Date: Tue, 04 Apr 2023 03:32:04 +0000 Gerrit-HasComments: Yes
