KeDeng has posted comments on this change. ( http://gerrit.cloudera.org:8080/19670 )
Change subject: [tablet] GC ancient, fully deleted rowsets witout live row count stats ...................................................................... Patch Set 8: (19 comments) Thanks for your reviews. 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: [tablet] GC ancient, fully deleted rowsets witout live row co > [tablet] GC ancient, fully deleted rowsets witout live row count stats Done http://gerrit.cloudera.org:8080/#/c/19670/7//COMMIT_MSG@10 PS7, Line 10: That > That Done http://gerrit.cloudera.org:8080/#/c/19670/7//COMMIT_MSG@12 PS7, Line 12: stats. A > stats Done http://gerrit.cloudera.org:8080/#/c/19670/7//COMMIT_MSG@13 PS7, Line 13: already > already existing Done 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: REDO delta stores per their : // delta stats. > nit: REDO delta stores per their delta stats Done 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 Done 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? Oh, just for the tidy test. 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: ASSERT_O > Here and below: why not to use ASSERT_OK() here? Done 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? Just for tidy test, the 'final' is redundant since the function is already declared 'override'. http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/diskrowset.h@387 PS7, Line 387: // Because possible operations in DMS and will be ignored, mainly because there is no API : // available in t > Do you mind adding more details what you meant by 'possible operations is D Done http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/diskrowset.h@389 PS7, Line 389: oximate live row count. > Maybe, rename to CountLiveRowsWithoutLiveRowCountStats() ? Done 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: tats. This is used to release " > This is used to release Done http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet.cc@225 PS7, Line 225: row count > row count stats Done http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet.cc@226 PS7, Line 226: generated " : "by Kudu > generated by Kudu clusters that do not have live row count stats Done 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 Done http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet_history_gc-test.cc@117 PS7, Line 117: change. > change Done http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet_history_gc-test.cc@120 PS7, Line 120: const int orig > nit: add 'const'? Done http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet_history_gc-test.cc@121 PS7, Line 121: const int orig > nit: add 'const'? Done http://gerrit.cloudera.org:8080/#/c/19670/7/src/kudu/tablet/tablet_history_gc-test.cc@751 PS7, Line 751: GC > nit: GC Done -- 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: 8 Gerrit-Owner: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> 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: Sat, 15 Apr 2023 04:54:21 +0000 Gerrit-HasComments: Yes
