Alexey Serbin 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 7: (19 comments) http://gerrit.cloudera.org:8080/#/c/19670/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19670/7//COMMIT_MSG@7 PS7, Line 7: [GC] gc ancient, fully deleted rowsets without live row count [tablet] GC ancient, fully deleted rowsets witout live row count stats http://gerrit.cloudera.org:8080/#/c/19670/7//COMMIT_MSG@10 PS7, Line 10: This That http://gerrit.cloudera.org:8080/#/c/19670/7//COMMIT_MSG@12 PS7, Line 12: features stats http://gerrit.cloudera.org:8080/#/c/19670/7//COMMIT_MSG@13 PS7, Line 13: inventory already existing http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/delta_tracker.h File src/kudu/tablet/delta_tracker.h: http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/delta_tracker.h@290 PS7, Line 290: the redo files whose with : // delta stats. nit: REDO delta stores per their delta stats http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/delta_tracker.cc File src/kudu/tablet/delta_tracker.cc: http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/delta_tracker.cc@1018 PS7, Line 1018: // We won't force open files just to read their stats. Could you also add corresponding note to the comment for this method in the .h file? http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/diskrowset-test-base.h File src/kudu/tablet/diskrowset-test-base.h: http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/diskrowset-test-base.h@a101 PS7, Line 101: Why this change? http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/diskrowset-test.cc File src/kudu/tablet/diskrowset-test.cc: http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/diskrowset-test.cc@780 PS7, Line 780: CHECK_OK Here and below: why not to use ASSERT_OK() here? http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/diskrowset.h File src/kudu/tablet/diskrowset.h: http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/diskrowset.h@a383 PS7, Line 383: Why is this change? http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/diskrowset.h@387 PS7, Line 387: // Because possible operations in DMS may ignored, this API can only obtain an approximate : // live row count Do you mind adding more details what you meant by 'possible operations is DMS may ignored'? http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/diskrowset.h@389 PS7, Line 389: CountLiveRowsWithoutLiveRowCount Maybe, rename to CountLiveRowsWithoutLiveRowCountStats() ? http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet.cc@225 PS7, Line 225: This feature mainly use to release This is used to release http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet.cc@225 PS7, Line 225: row count row count stats http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet.cc@226 PS7, Line 226: in the old Kudu " : "cluster generated by Kudu clusters that do not have live row count stats http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet_history_gc-test.cc File src/kudu/tablet/tablet_history_gc-test.cc: http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet_history_gc-test.cc@116 PS7, Line 116: gc GC http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet_history_gc-test.cc@117 PS7, Line 117: changes change http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet_history_gc-test.cc@120 PS7, Line 120: int orig_bytes nit: add 'const'? http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet_history_gc-test.cc@121 PS7, Line 121: int orig_count nit: add 'const'? http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet_history_gc-test.cc@751 PS7, Line 751: gc nit: GC -- 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: 7 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Fri, 14 Apr 2023 14:54:12 +0000 Gerrit-HasComments: Yes
