Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )
Change subject: [fs] add metrics for untracked orphaned blocks ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet.cc@1741 PS2, Line 1741: TabletMetadata::OrphanBlockCleanupStats orphan_stats; : RETURN_NOT_OK(metadata_->Flush(&orphan_stats)); : UpdateOrphanBlockMetrics(orphan_stats); > Here and elsewhere: why not to encapsulate this into TabletMetadata::Flush That would involve passing TabletMetrics* to TabletMetadata so that callee can set it within the method where orphan stats are populated. This creates a dependency on metrics within metadata. A cleaner way would be to use a function callback but that adds more infra changes just to avoid these 3 lines. Repetition is a mild inconvenience over other changes that are either adding more infra changes or introducing coupling of TabletMetaData to TabletMetrics. http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet.cc@1986 PS2, Line 1986: if (!metrics_) return; > style nit: add braces scope even for one-line 'if' scope, similar to one-li Done http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet.cc@1988 PS2, Line 1988: metrics_->orphaned_block_cleanup_failures->IncrementBy(stats.blocks_failed); : } > Are these metrics updated multiple times with the same participant blocks w No, the metrics are updated the first time failures only. Since we are erasing all the blockids from orphaned_blocks_, there is no delete retry on the blocks for which failure was seen the first time. However, your point is applicable to online GC op (https://gerrit.cloudera.org/#/c/24125) where information of block ids with failed deletions is kept in super block for retry. Anyway, I have to incorporate these changes into online GC patch. I will make a note to address this in that patch. This went out for review well before online GC patch with the expectation that this would get merged first followed by the online GC one where code reconciliation/rebase is going to happen. -- To view, visit http://gerrit.cloudera.org:8080/24092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9 Gerrit-Change-Number: 24092 Gerrit-PatchSet: 3 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Thu, 26 Mar 2026 12:49:11 +0000 Gerrit-HasComments: Yes
