Yingchun Lai 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 5:

(6 comments)

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: TestCountLiveRowsWithoutLiveRowCount
I think it's necessary to test that when the rows of this DRS have been fully 
deleted, CountLiveRows is able to get 0, and CountLiveRowsWithoutLiveRowCount 
is able to get a value in [0, inserted_row_count], and it would be nice to get 
a value less than inserted_row_count.


http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc@773
PS5, Line 773:   WriteTestRowSet();
The function declaration is

 void WriteTestRowSet(int n_rows = 0, bool zero_vals = false)

Is it what you want to write 0 rows to the rowset?


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'


http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc@790
PS5, Line 790:   CHECK_OK(rs->CountLiveRows(&rs_live_rows));
nit: Check 'rs_live_rows' is not change after some update operations.


http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc@801
PS5, Line 801: ASSERT_LE(rs_live_rows_without_lrc, rs_live_rows);
Also to check that 'rs_live_rows_without_lrc' is larger than some value?


http://gerrit.cloudera.org:8080/#/c/19670/5/src/kudu/tablet/diskrowset-test.cc@802
PS5, Line 802: MajorCompactDeltaStores
I'm curious what's the purpose of this compact in this test?



--
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: 5
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: Mon, 03 Apr 2023 16:11:27 +0000
Gerrit-HasComments: Yes

Reply via email to