Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15145 )
Change subject: KUDU-1625: background op to GC ancient, fully deleted rowsets ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/15145/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15145/1//COMMIT_MSG@12 PS1, Line 12: no live rows, that rowset will be deleted. : : It'd be nice if we > My concern is around rebuilding the DMS from the WAL. I didn't look very ha Upon further inspection, there seems to have been nothing to worry about. The "missing DRS" on bootstrap today eg if we compact away a DRS and try to replay ops to a DMS that belonged to that DRS. http://gerrit.cloudera.org:8080/#/c/15145/1//COMMIT_MSG@15 PS1, Line 15: with > Probably meant 'earlier', no? Earlier/older, yep! http://gerrit.cloudera.org:8080/#/c/15145/1/src/kudu/tablet/delta_tracker.cc File src/kudu/tablet/delta_tracker.cc: http://gerrit.cloudera.org:8080/#/c/15145/1/src/kudu/tablet/delta_tracker.cc@457 PS1, Line 457: return *dms_highest_timestamp < ancient_history_mark; > Can newest_redo be null? If not, enforce via DCHECK? It can, added that to the condition. http://gerrit.cloudera.org:8080/#/c/15145/1/src/kudu/tablet/memrowset.h File src/kudu/tablet/memrowset.h: http://gerrit.cloudera.org:8080/#/c/15145/1/src/kudu/tablet/memrowset.h@400 PS1, Line 400: return Status::OK(); > While we could theoretically determine whether all the mutations in the MRS Yeah, I suppose I could have called this 'empty_and_ancient_and_gcable' given we only care about it if we can GC some bytes. Left this as is though, unless you think the current handling is confusing. http://gerrit.cloudera.org:8080/#/c/15145/1/src/kudu/tablet/tablet_mm_ops.h File src/kudu/tablet/tablet_mm_ops.h: http://gerrit.cloudera.org:8080/#/c/15145/1/src/kudu/tablet/tablet_mm_ops.h@162 PS1, Line 162: class DeletedRowsetGCOp : public TabletOpBase { > "Empty" suggests a rowset without rows, but this is actually a rowset whose Went with just "Deleted" -- To view, visit http://gerrit.cloudera.org:8080/15145 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I696e2a29ea52ad4e54801b495c322bc371787124 Gerrit-Change-Number: 15145 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin <verjov...@cloudera.com> Gerrit-Comment-Date: Tue, 17 Mar 2020 06:06:44 +0000 Gerrit-HasComments: Yes
