You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Ashwani Raina (Code Review)" <ge...@cloudera.org> on 2026/03/12 12:50:57 UTC

[kudu-CR] [WIP] [fs] keep orphaned blocks to cleanup in next flush

Ashwani Raina has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24092

Change subject: [WIP] [fs] keep orphaned blocks to cleanup in next flush
......................................................................

[WIP] [fs] keep orphaned blocks to cleanup in next flush

During flush, if orphaned blocks are there and for some reason those
blocks' records are not deleted (maybe due to some underlying transient
I/O error, etc), those blocks are never re-tried for deletion and erased
from the list of orphaned_blocks_. This can cause un-reclaimed space on
the persistent storage that can get accumulated over time to a big size.
This un-reclaimed space can only be reclaimed with user intervention by
running 'kudu fs check --repair'.

This patch addresses the issue by not removing those orphaned blocks
from the orphaned_blocks_ list, for which delete failed due to some
transient error. This ensures that when next flush is scheduled, the
metadata still contains those orphaned blocks in the list and delete
can be retried on those blocks to reclaim the space.

Highlights of the change:
- Add warning logs at CommitDeletedBlocks callers when the commit fails.
- Add per-block level error logs for blocks for which deletion record
could not be committed.
- Add these metrics to hold the orphan block deletion outcome:
* orphaned_blocks_cleaned
* orphaned_block_cleanup_failures
* orphaned_block_cleanup_failures_bytes
- Add unit tests to test three scenarios with metrics verification:
* Usual path where orphaned blocks are deleted with no error
* Induced I/O error where orphaned blocks are not deleted
* Two stages: First induce I/O error, verify metrics shows cleanup
failure count increased. Second remove induced error, verify metrics
shows cleaned count increased.

TODO:
1. Investigate KUDU-2635 angle for assumption in tablet deletion code
to expect orphan list be empty. If possible, run orphaned block
cleanup as part of tablet deletion workflow. Remedy to cleanup those
orphan blocks (for which deletion failed earlier) and tablet deletion
is bound to happen.
2. If adding the failed orphan blocks back to the list is not an option,
repurpose this patch to add metrics and necessary logging for better
diagnosis of scenarios where space runs out in orphaned blocks.
3. As part of this patch or separate one, add a background op to reclaim
the space left from orphaned blocks (KUDU-829)

Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
---
M src/kudu/fs/log_block_manager.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_metrics.h
8 files changed, 397 insertions(+), 33 deletions(-)

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/24092/1
--
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: newchange
Gerrit-Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
Gerrit-Change-Number: 24092
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 8:

Build Started http://jenkins.kudu.apache.org/job/pre_commit/2513/


-- 
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: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Apr 2026 14:04:46 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

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 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3417
PS5, Line 3417:     LOG(ERROR) << Substitute(
> Thanks for the explanation.  I guess it means you consider this as related 
Done.
Separate patch here: https://gerrit.cloudera.org/#/c/24228/
Rebased on top of this change.


http://gerrit.cloudera.org:8080/#/c/24092/7/src/kudu/util/logging.h
File src/kudu/util/logging.h:

PS7: 
> Please don't avoid piling up logically non-related changes in a single chan
Done.
Separate patch here: https://gerrit.cloudera.org/#/c/24228/
Rebased on top of this change.



-- 
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: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Apr 2026 14:15:30 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

Hello Marton Greber, Alexey Serbin, Zoltan Martonka, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/24092

to look at the new patch set (#6).

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................

[fs] add metrics for untracked orphaned blocks

Initially, this patch addressed the issue by not removing those orphaned
blocks from the orphaned_blocks_ list, for which delete failed due to
some transient error. This ensures that when next flush is scheduled,
the metadata still contains those orphaned blocks in the list and delete
can be retried on those blocks to reclaim the space.

However, with that change, TestEIODuringDelete started failing because
DeleteTabletData expects an empty orphaned_blocks_ set after calling
Flush() couple of times. A second flush is required to clear the
orphaned blocks from superblock even if delete failed for those earlier.
Since we keep adding the orphaned blocks (for which delete failed)
back to the set, it never got an empty set, that caused the failure.

If orphaned_blocks_ set is cleared after the second flush, to maintain
original behavior and expectation, it may fix the TestEIODuringDelete
failure but it opens the possibility of hitting KUDU-1060 where a number
tombstoned tablets keep the record of all orphaned blocks on persistent
superblock i.e., roll-forward of the block deletions until next restart.
This can severely impact the startup time for a tablet server that has
a lot of tombstoned tablets.

In a nutshell, having to keep these orphaned blocks in the set may not
be of much use if the disk error is persistent and not to mention the
additional handling required in various cases like mentioned above. It
makes sense to just rely on 'kudu fs check --repair' like workflow to
remove these stale orphaned blocks as a maintenance operation.

With all this information, re-purposing this patch to only focus on
adding additional logs, metrics and stats that can help identify the
scenarios with stale orphaned block id lists and log the appropriate
action for user i.e. 'kudu fs check --repair'. Original behavior holds
for orphaned blocks set i.e. erase all blocks from the set irrespective
of the CommitDeletedBlocks() outcome.

Follow-up:
KUDU-829 - Create a separate patch for adding a maintenance op that can
run in the background to reclaim all the space left from those orphaned
blocks using the same logic as 'kudu fs check --repair'.

Highlights of the change:
- Add warning logs at CommitDeletedBlocks callers when the commit fails.
- Add per-block level error logs for blocks for which deletion record
could not be committed.
- Add these metrics that can hold the orphan block deletion outcome:
* orphaned_blocks_cleaned
* orphaned_block_cleanup_failures
- Add unit tests to test these scenarios with metrics verification:
* Usual path where orphaned blocks are deleted with no error.
* No metrics are updated when orphan block deletion is disabled.
* Induced I/O error causes orphaned blocks lying around that are not
deleted and eventually removed from the set with a action to user.
* Two stages: First induce I/O error, verify metrics shows cleanup
failure count increased. Second remove induced error, verify metrics
remains unchanged.

Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
---
M src/kudu/fs/log_block_manager.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_metrics.h
M src/kudu/util/logging.h
9 files changed, 375 insertions(+), 38 deletions(-)

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/24092/6
--
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: newpatchset
Gerrit-Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
Gerrit-Change-Number: 24092
Gerrit-PatchSet: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 5:

(16 comments)

http://gerrit.cloudera.org:8080/#/c/24092/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24092/5//COMMIT_MSG@35
PS5, Line 35: severly
severely


http://gerrit.cloudera.org:8080/#/c/24092/5//COMMIT_MSG@53
PS5, Line 53: backgrounnd
background


http://gerrit.cloudera.org:8080/#/c/24092/5//COMMIT_MSG@57
PS5, Line 57: warning
For some reason, those are ERROR logs, not WARNING.  Is it intentional?


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@a3343
PS5, Line 3343: 
This comment is removed in PS5, but the corresponding metrics aren't present still.  IIUC, the only added metrics track failures of the blocks' GC-ing, but the blocks that failed to be added into the metadata intent records aren't tracked as of PS5.


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3350
PS5, Line 3350:       // Blocks whose deletion metadata could not be written are permanently
              :       // orphaned i.e., their data occupies space on disk. They will not be cleaned
              :       // up automatically and may cause unclaimed disk space accumulation over time.
Shouldn't we track those using metrics as well?


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3357
PS5, Line 3357: int
Can we ever expect an overflow here with 32-bit integers?


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3360
PS5, Line 3360:             "Block $0 in container $1 could not have its deletion record "
              :             "committed and is now an orphaned block that occupies "
              :             "untracked disk space: $2. Run 'kudu fs check --repair' to "
              :             "reclaim this disk space.
Could you make this message more concise, but still actionable?


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3366
PS5, Line 3366:       // Always emit a summary so the total orphaned count is never suppressed,
              :       // even when the per-block messages above are rate-limited.
What if there are thousands of containers there since that was a failed data directory?


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3369
PS5, Line 3369:           "$0 block(s) in container $1 could not have their deletion records "
              :           "committed and are now orphaned blocks occupying unreclaimed disk space: "
              :           "$2. Run 'kudu fs check --repair' to reclaim this disk space.",
nit: make this more concise, but still actionable.


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3417
PS5, Line 3417:       KLOG_EVERY_N_SECS(INFO, 1) << Substitute("Block $0 is in a failed directory; not deleting",
Is this update also related to this patch?  If not, avoid introducing such updates since these increase risk of textual conflicts when trying to cherry-pick/merge logically independent patches.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata-test.cc
File src/kudu/tablet/tablet_metadata-test.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata-test.cc@454
PS2, Line 454:     // therefore unaffected. Also, Tablet::Flush() still returns OK overall.
nit: if possible, please avoid updating non-related code since it introduces higher risk of textual conflicts when cherry-picking or merging patches


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc@562
PS5, Line 562: Scheduling $0 block(s) for orphaned cleanup
nit: ... scheduling $0 orphaned block(s) for cleanup ... ?


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc@575
PS5, Line 575:     LOG_WITH_PREFIX(WARNING) << "Not deleting " << blocks.size()
             :         << " orphaned block(s) from disk. Block deletion disabled via "
             :         << "--enable_tablet_orphaned_block_deletion=false";
nit: if updating this message, please switch to using Substitute() here as well


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc@581
PS5, Line 581:   OrphanBlockCleanupStats stats;
nit: move this closer to the place of the usage


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc@595
PS5, Line 595: ERROR
There used to be a log message of WARNING priority.  Is the change to the ERROR priority intentional?


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc@596
PS5, Line 596:         "Failed to clean up $0/$1 orphaned block(s): $2. The failed block(s) are removed from "
             :         "the superblock but their disk space may be unreclaimed. Run 'kudu fs check --repair' "
             :         "to manually reclaim the disk space.
nit: please make it more concise while keeping it actionable, e.g.

"$2: failed cleaning up $0 of $1 removed blocks; running 'kudu fs check --repair' can help to reclaim disk space"



-- 
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: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Apr 2026 22:20:40 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 5:

Build Started http://jenkins.kudu.apache.org/job/pre_commit/2427/


-- 
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: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Mar 2026 10:49:39 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 6:

Build Aborted 

http://jenkins.kudu.apache.org/job/pre_commit/2458/ : ABORTED


-- 
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: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Apr 2026 06:35:06 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 4:

Build Started http://jenkins.kudu.apache.org/job/pre_commit/2425/


-- 
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: 4
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Mar 2026 12:54:54 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc@234
PS2, Line 234: Failed blocks are retried on the next metadata flush.
> If this means blocks can be eventually cleaned up during lifecycle of a tab
From the code it seems this is indeed a counter given the argument for IncrementBy() is always non-negative.  I was a bit confused by the signed integer type for OrphanBlockCleanupStats::blocks_failed field and this comment.

With that, please ignore the comment above.



-- 
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: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Mar 2026 20:48:53 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 5: Verified-1

Build Failed 

http://jenkins.kudu.apache.org/job/pre_commit/2450/ : FAILURE


-- 
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: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Apr 2026 10:49:16 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

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

(16 comments)

http://gerrit.cloudera.org:8080/#/c/24092/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24092/5//COMMIT_MSG@35
PS5, Line 35: severly
> severely
Done


http://gerrit.cloudera.org:8080/#/c/24092/5//COMMIT_MSG@53
PS5, Line 53: backgrounnd
> background
Done


http://gerrit.cloudera.org:8080/#/c/24092/5//COMMIT_MSG@57
PS5, Line 57: warning
> For some reason, those are ERROR logs, not WARNING.  Is it intentional?
Changed back to WARNING as it was before.


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@a3343
PS5, Line 3343: 
> This comment is removed in PS5, but the corresponding metrics aren't presen
There is no GC'ing logic present in PS5. So, any block for which deletion failed, it will be accounted for in OrphanBlockCleanupStats.blocks_failed which gets updated in orphaned_block_cleanup_failures metric.

In other words, if deletion failed for a set of blocks due to any I/O error, metrics orphaned_block_cleanup_failures represents the number of such orphaned blocks that couldn't be cleaned up.

An orphaned block is the one which doesn't have any entry in metadata and this is an short-lived intermediate stage of blocks (to be deleted) before these get deleted in normal workflow (without any GC in picture). Generally, you won't see any orphaned blocks in a kudu cluster which is running fine and doesn't face any issues. It is only when this intermediate stage gets converted into a permanent state of the orphaned blocks because regular clean-up workflow failed for them. This is represented by orphaned_block_cleanup_failures metric.

GC'ing is an additional logic on top of this ever existing clean-up code (tracked under https://gerrit.cloudera.org/#/c/24125)

orphaned_block_cleanup_failures represents the metric to track the number of long-living orphaned blocks which is what TODO is talking about.


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3350
PS5, Line 3350:       // Blocks whose deletion metadata could not be written are permanently
              :       // orphaned i.e., their data occupies space on disk. They will not be cleaned
              :       // up automatically and may cause unclaimed disk space accumulation over time.
> Shouldn't we track those using metrics as well?
This is not different from what is being tracked under orphaned_block_cleanup_failures metrics. If you see the callers in the cleanup path context, Tablet::FlushMetadata makes use of deleted_block_ids (in the form of OrphanBlockCleanupStats::blocks_cleaned and OrphanBlockCleanupStats::blocks_failed).


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3357
PS5, Line 3357: int
> Can we ever expect an overflow here with 32-bit integers?
Changed to size_t, aligns with size() return type.


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3360
PS5, Line 3360:             "Block $0 in container $1 could not have its deletion record "
              :             "committed and is now an orphaned block that occupies "
              :             "untracked disk space: $2. Run 'kudu fs check --repair' to "
              :             "reclaim this disk space.
> Could you make this message more concise, but still actionable?
Done


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3366
PS5, Line 3366:       // Always emit a summary so the total orphaned count is never suppressed,
              :       // even when the per-block messages above are rate-limited.
> What if there are thousands of containers there since that was a failed dat
Made some modification to time-limit these messages as well and have a aggregate message outside the loop.


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3369
PS5, Line 3369:           "$0 block(s) in container $1 could not have their deletion records "
              :           "committed and are now orphaned blocks occupying unreclaimed disk space: "
              :           "$2. Run 'kudu fs check --repair' to reclaim this disk space.",
> nit: make this more concise, but still actionable.
Done


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3417
PS5, Line 3417:       KLOG_EVERY_N_SECS(INFO, 1) << Substitute("Block $0 is in a failed directory; not deleting",
> Is this update also related to this patch?  If not, avoid introducing such 
In any case, this needs to change because src/kudu/util/logging.h (included in this file) undefs LOG_EVERY_N and throws following  compilation error:
++
GOOGLE_GLOG_COMPILE_ASSERT(false, "LOG_EVERY_N is deprecated. Please use KLOG_EVERY_N.")
++

Alternatively, I can change this to:
++
KLOG_EVERY_N(INFO, 10) << Substitute("Block $0 is in a failed directory; not deleting", block_id.ToString());
++
However, clang throws 'readability-isolate-declaration' warning for this because KLOG_EVERY_N expand to KUDU_SOME_KIND_OF_LOG_EVERY_N macros which has two declaration in a statement i.e.
++
static int LOG_OCCURRENCES = 0, LOG_OCCURRENCES_MOD_N = 0; \
++

I have changed to KLOG_EVERY_N, along with changes in src/kudu/util/logging.h to ensure single declaration in a statement and some other changes as well. Frankly, it is too many adjustments for just a single line change.

All of this circus goes away if time-based logging is chosen. Not to mention the additional benefit of predictable log bounding.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata-test.cc
File src/kudu/tablet/tablet_metadata-test.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata-test.cc@454
PS2, Line 454:     // therefore unaffected. Also, Tablet::Flush() still returns OK overall.
> nit: if possible, please avoid updating non-related code since it introduce
Not sure what non-related code you are talking about. This is a newly added unit test to verify the behavior by injecting EIO on every operation under this directory, a requirement that is related to this patch.


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc@562
PS5, Line 562: Scheduling $0 block(s) for orphaned cleanup
> nit: ... scheduling $0 orphaned block(s) for cleanup ... ?
Done


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc@575
PS5, Line 575:     LOG_WITH_PREFIX(WARNING) << "Not deleting " << blocks.size()
             :         << " orphaned block(s) from disk. Block deletion disabled via "
             :         << "--enable_tablet_orphaned_block_deletion=false";
> nit: if updating this message, please switch to using Substitute() here as 
Done


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc@581
PS5, Line 581:   OrphanBlockCleanupStats stats;
> nit: move this closer to the place of the usage
Done


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc@595
PS5, Line 595: ERROR
> There used to be a log message of WARNING priority.  Is the change to the E
Changed to WARNING


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/tablet/tablet_metadata.cc@596
PS5, Line 596:         "Failed to clean up $0/$1 orphaned block(s): $2. The failed block(s) are removed from "
             :         "the superblock but their disk space may be unreclaimed. Run 'kudu fs check --repair' "
             :         "to manually reclaim the disk space.
> nit: please make it more concise while keeping it actionable, e.g.
Done



-- 
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: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Apr 2026 06:28:48 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

Hello Marton Greber, Alexey Serbin, Zoltan Martonka, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/24092

to look at the new patch set (#3).

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................

[fs] add metrics for untracked orphaned blocks

If orphaned_blocks_ set is cleared after the second flush, to maintain
original behavior and expectation, it may fix the TestEIODuringDelete
failure but it opens the possibility of hitting KUDU-1060 where a number
tombstoned tablets keep the record of all orphaned blocks on persistent
superblock i.e., roll-forward of the block deletions until next restart.
This can severly impact the startup time for a tablet server that has
a lot of tombstoned tablets.

Follow-up:
KUDU-829 - Create a separate patch for adding a maintenance op that can
run in the backgrounnd to reclaim all the space left from those orphaned
blocks using the same logic as 'kudu fs check --repair'.

Highlights of the change:
- Add warning logs at CommitDeletedBlocks callers when the commit fails.
- Add per-block level error logs for blocks for which deletion record
could not be committed.
- Add these metrics that can hold the orphan block deletion outcome:
* orphaned_blocks_cleaned
* orphaned_block_cleanup_failures
- Add unit tests to test these scenarios with metrics verification:
* Usual path where orphaned blocks are deleted with no error.
* No metrics are updated when orphan block deletion is disabled.
* Induced I/O error causes orphaned blocks lying around that are not
deleted and eventually removed from the set with a action to user.
* Two stages: First induce I/O error, verify metrics shows cleanup
failure count increased. Second remove induced error, verify metrics
remains unchanged.

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/24092/3
--
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: newpatchset
Gerrit-Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
Gerrit-Change-Number: 24092
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/24092

to look at the new patch set (#2).

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................

[fs] add metrics for untracked orphaned blocks

During flush, if orphaned blocks are there and for some reason those
blocks' records are not deleted (maybe due to some underlying transient
I/O error, etc), those blocks are never re-tried for deletion and erased
from the list of orphaned_blocks_. This can cause un-reclaimed space on
the persistent storage that can get accumulated over time to a big size.
This un-reclaimed space can only be reclaimed with user intervention by
running 'kudu fs check --repair'.

Initially, this patch addressed the issue by not removing those orphaned
blocks from the orphaned_blocks_ list, for which delete failed due to
some transient error. This ensures that when next flush is scheduled,
the metadata still contains those orphaned blocks in the list and delete
can be retried on those blocks to reclaim the space.

However, with that change, TestEIODuringDelete started failing because
DeleteTabletData expects an empty orphaned_blocks_ set after calling
Flush() couple of times. A second flush is required to clear the
orphaned blocks from superblock even if delete failed for those earlier.
Since we keep adding the orphaned blocks (for which delete failed)
back to the set, it never got an empty set, that caused the failure.

If orphaned_blocks_ set is cleared after the second flush, to maintain
original behavior and expectation, it may fix the TestEIODuringDelete
failure but it opens the possibility of hitting KUDU-1060 where a number
tombstoned tablets keep the record of all orphaned blocks on persistent
superblock i.e., roll-forward of the block deletions until next restart.
This can severly impact the startup time for a tablet server that has
a lot of tombstoned tablets.

In a nutshell, having to keep these orphaned blocks in the set may not
be of much use if the disk error is persistent and not to mention the
additional handling required in various cases like mentioned above. It
makes sense to just rely on 'kudu fs check --repair' like workflow to
remove these stale orphaned blocks as a maintenance operation.

With all this information, re-purposing this patch to only focus on
adding additional logs, metrics and stats that can help identify the
scenarios with stale orphaned block id lists and log the appropriate
action for user i.e. 'kudu fs check --repair'. Original behavior holds
for orphaned blocks set i.e. erase all blocks from the set irrespective
of the CommitDeletedBlocks() outcome.

Follow-up:
KUDU-829 - Create a separate patch for adding a maintenance op that can
run in the backgrounnd to reclaim all the space left from those orphaned
blocks using the same logic as 'kudu fs check --repair'.

Highlights of the change:
- Add warning logs at CommitDeletedBlocks callers when the commit fails.
- Add per-block level error logs for blocks for which deletion record
  could not be committed.
- Add these metrics that can hold the orphan block deletion outcome:
  * orphaned_blocks_cleaned
  * orphaned_block_cleanup_failures
  * orphaned_block_cleanup_failures_bytes
- Add unit tests to test these scenarios with metrics verification:
  * Usual path where orphaned blocks are deleted with no error.
  * No metrics are updated when orphan block deletion is disabled.
  * Induced I/O error causes orphaned blocks lying around that are not
    deleted and eventually removed from the set with a action to user.
  * Two stages: First induce I/O error, verify metrics shows cleanup
    failure count increased. Second remove induced error, verify metrics
    remains unchanged.

Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
---
M src/kudu/fs/log_block_manager.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_metrics.h
8 files changed, 396 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/24092/2
-- 
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: newpatchset
Gerrit-Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
Gerrit-Change-Number: 24092
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

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:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@2528
PS2, Line 2528:   return deletion_transaction->CommitDeletedBlocks(nullptr);
> Why not to move this report/warning piece closer to the source of the non-O
Actually, RemoveLogBlocks already has such log message that makes this log redundant. Removed.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@2533
PS2, Line 2533: 
> There might be many blocks failing to delete if they are on a failed data d
This log message is removed now so the comment is not applicable here but it is definitely applicable to the similar log message in RemoveLogBlocks().

Have applied rate limit to the log message in RemoveLogBlocks().


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@2534
PS2, Line 2534: 
> I'm not sure this is always due to a failure writing metadata: e.g., what a
No applicable now that whole log message has been removed.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.h
File src/kudu/tablet/tablet_metadata.h:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.h@189
PS2, Line 189:   struct OrphanBlockCleanupStats {
             :     // Number of orphaned blocks successfully deleted from dis
> nit: why to mention this here at all when this belongs rather to docs on pa
Removed.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.h@192
PS2, Line 192:     // Number of orphaned blocks that could not be deleted (e.g. I/O error).
             :     // These are removed from the superblock regardless (see DeleteOrphanedBlocks),
             :     // so any unreclaimed disk space must be recovered via 'kudu fs check --repair'.
             :     int64_t blocks_failed = 0;
             :   };
> What's the reason behind using signed integers for counters when using unsi
Instead of 'int', it should be int64_t to make this exactly same as input parameter data type of IncrementBy() method, so no implicit conversion is required. That also aligns with what is done at other places in the code (e.g. tablet_bytes_deleted).


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc@565
PS2, Line 565: begin(), block_ids.end());
> nit: remove the enclosing parentheses?
Done


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc@572
PS2, Line 572: }
> style nit: always use scope braces for 'if', similar to 'for' cycle
Done


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h
File src/kudu/tablet/tablet_metrics.h:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h@87
PS2, Line 87: underlying error.
> nit: does the related code even capable of differentiating between I/O and 
Not really. Replaced with:
"Number of orphaned blocks that could not be deleted due to  underlying errors."


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h@88
PS2, Line 88: ped_refptr<Counter> orphaned_block_cleanup_failures
> This is confusing.  Sustained by which means?  These counters are reset to 
This is a leftover from PS1 where orphaned blocks were kept in the set for next flush call to retry the delete on those.   PS2 removed that logic and made this change only address the non-functional entity of adding metrics and no change in behavior.

However, even with PS1, using 'sustained' might sound confusing. I have removed this line to avoid any ambiguity and keep the purpose of this metric clear.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h@92
PS2, Line 92: > delta_file_lookups_per_op;
> If introducing this pairing metric for orphaned_block_cleanup_failures, why
I have removed this metric because of the reason stated here:
https://gerrit.cloudera.org/#/c/24092/2/src/kudu/tablet/tablet_metadata-test.cc@441


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h@94
PS2, Line 94:   scoped_refptr<Histogram> commit_wait_duration;
> Does it make sense to introduce a gauge to reflect on the size of the block
It would certainly align with the semantics of data_retained_bytes_ in maintenance manager. I will try to address this in 24125 patch.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc@241
PS2, Line 241: if the tablet is not fully compacted. High frequency of "
             :                         "high values may indicate that compaction is falling behind.",
             :                         kudu::MetricLevel::kDebug,
             :                         20, 2);
> Move this out and merge into the description of the orphaned_block_cleanup_
I have removed orphaned_block_cleanup_failures_bytes metric altogether. Refer to this comment for more details:
https://gerrit.cloudera.org/#/c/24092/2/src/kudu/tablet/tablet_metadata-test.cc@441


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc@245
PS2, Line 245: 
> FWIW, it's already enough to have orphaned_block_cleanup_failures as a metr
Same response as stated above.



-- 
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 <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Mar 2026 11:14:03 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 5: -Verified

Build Started http://jenkins.kudu.apache.org/job/pre_commit/2450/


-- 
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: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Apr 2026 09:55:06 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

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 <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Mar 2026 12:49:11 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

Hello Marton Greber, Alexey Serbin, Zoltan Martonka, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/24092

to look at the new patch set (#8).

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................

[fs] add metrics for untracked orphaned blocks

Highlights of the change:
- Add warning logs at CommitDeletedBlocks callers when the commit fails.
- Add per-block level error logs for blocks for which deletion record
could not be committed.
- Add these metrics that can hold the orphan block deletion outcome:
* orphaned_blocks_cleaned
* orphaned_block_cleanup_failures
- Add unit tests to test these scenarios with metrics verification:
* Usual path where orphaned blocks are deleted with no error.
* No metrics are updated when orphan block deletion is disabled.
* Induced I/O error causes orphaned blocks lying around that are not
deleted and eventually removed from the set with a action to user.
* Two stages: First induce I/O error, verify metrics shows cleanup
failure count increased. Second remove induced error, verify metrics
remains unchanged.

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/24092/8
--
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: newpatchset
Gerrit-Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
Gerrit-Change-Number: 24092
Gerrit-PatchSet: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 2:

Build Started http://jenkins.kudu.apache.org/job/pre_commit/2372/


-- 
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: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 13 Mar 2026 13:38:18 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 3: Verified+1

Build Successful 

http://jenkins.kudu.apache.org/job/pre_commit/2424/ : SUCCESS


-- 
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 <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Mar 2026 11:49:29 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 3:

Build Started http://jenkins.kudu.apache.org/job/pre_commit/2424/


-- 
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 <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Mar 2026 10:59:19 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.

Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 5: Code-Review+2

LGTM, triggered a fresh build as the old one expired.


-- 
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: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Apr 2026 09:55:29 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 2: Verified-1

Build Failed 

http://jenkins.kudu.apache.org/job/pre_commit/2372/ : FAILURE


-- 
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: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Mar 2026 14:18:29 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 2:

(2 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@1988
PS2, Line 1988:   metrics_->orphaned_block_cleanup_failures->IncrementBy(stats.blocks_failed);
              :   metrics_->orphaned_block_cleanup_failures_bytes->IncrementBy(stats.bytes_failed);
Are these metrics updated multiple times with the same participant blocks when retrying deletion of the blocks that failed to be deleted previous run of the tablet's metadata flush?  If so, this might become confusing and end up in inflated and scary metrics.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h
File src/kudu/tablet/tablet_metrics.h:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h@94
PS2, Line 94:   scoped_refptr<Histogram> bloom_lookups_per_op;
Does it make sense to introduce a gauge to reflect on the size of the blocks currently pending removal in addition to this accumulating counter?  It might be used instead in https://gerrit.cloudera.org/#/c/24125 for setting retained_bytes stats for the corresponding maintenance op.



-- 
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: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Mar 2026 23:01:32 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 7: Verified-1

Build Failed 

http://jenkins.kudu.apache.org/job/pre_commit/2459/ : FAILURE


-- 
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: 7
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Apr 2026 07:36:33 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.

Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 5: Verified+1

unrelated test failure:
debug: dynamic_multi_master-test.3


-- 
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: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Apr 2026 10:54:59 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.

Marton Greber has removed a vote on this change.

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
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: deleteVote
Gerrit-Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
Gerrit-Change-Number: 24092
Gerrit-PatchSet: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.

Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@2529
PS2, Line 2529: help
nit: held ?


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@2530
PS2, Line 2530: lyring
nit: lying ?


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@3363
PS2, Line 3363:       // RemoveBlockIdsFromMetadata writes records in the same order as clbs,
Does this assumption hold for LogBlockContainerRdbMeta?


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata-test.cc
File src/kudu/tablet/tablet_metadata-test.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata-test.cc@441
PS2, Line 441: TEST_F(TestTabletMetadata, TestOrphanBlockDeletionFailed) {
Can we also assert orphaned_block_cleanup_failures_bytes in this test?


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc@601
PS2, Line 601:     auto bm_size = fs_manager()->block_manager();
do we need this declaration here? can't we reuse the above declared bm?


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc@623
PS2, Line 623:   // Regardless of whether we deleted all the blocks or not, remove them from
             :   // the orphaned blocks list. If we failed to delete the blocks due to
             :   // hardware issues, there's not much we can do and we assume the disk isn't
             :   // coming back. At worst, this leaves some untracked orphaned blocks.
Maybe I dont have the complete picture yet, but doesnt this contradict that above LOG_WITH_PREDIX message about "...will be retried on the next metadata flush..."


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc@247
PS2, Line 247: 
nit: extra new line?



-- 
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: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Mar 2026 12:36:57 +0000
Gerrit-HasComments: Yes

[kudu-CR] [WIP] [fs] keep orphaned blocks to cleanup in next flush

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [WIP] [fs] keep orphaned blocks to cleanup in next flush
......................................................................


Patch Set 1:

Build Started http://jenkins.kudu.apache.org/job/pre_commit/2364/


-- 
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: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 12 Mar 2026 12:51:03 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

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 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24092/7/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/7/src/kudu/fs/log_block_manager.cc@3414
PS7, Line 3414: lbs_by_containers.size());
> Does logging the container id (maybe under VLOG(1)) makes sense to help dur
Isn't that already in container path (i.e. ToString() output) as the last dirent?
For example, for this container path:
/data/kudu/data/2dd8f3ad2faa4aea88a25b7e81e1e7fe 

2dd8f3ad2faa4aea88a25b7e81e1e7fe is the container id.

If you are asking about adding container id in this log as well, 
I guess that wouldn't be necessary because this is already captured above at line numbers: 3389-3392 and 3394-3398.

Maybe having container path (or id) in this log instead above two would make a case but I want to emit the container path at regular interval in order to capture the culprit path while iterating over the blocks instead of collecting all the container paths from the inner loops and logging all of those at the end.

I hope that makes sense.



-- 
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: 7
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Apr 2026 07:15:27 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.

Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24092/7/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/7/src/kudu/fs/log_block_manager.cc@3414
PS7, Line 3414: lbs_by_containers.size());
Does logging the container id (maybe under VLOG(1)) makes sense to help during troubleshooting?



-- 
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: 7
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Wed, 15 Apr 2026 18:26:02 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

Hello Marton Greber, Alexey Serbin, Zoltan Martonka, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/24092

to look at the new patch set (#5).

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................

[fs] add metrics for untracked orphaned blocks

If orphaned_blocks_ set is cleared after the second flush, to maintain
original behavior and expectation, it may fix the TestEIODuringDelete
failure but it opens the possibility of hitting KUDU-1060 where a number
tombstoned tablets keep the record of all orphaned blocks on persistent
superblock i.e., roll-forward of the block deletions until next restart.
This can severly impact the startup time for a tablet server that has
a lot of tombstoned tablets.

Highlights of the change:
- Add warning logs at CommitDeletedBlocks callers when the commit fails.
- Add per-block level error logs for blocks for which deletion record
could not be committed.
- Add these metrics that can hold the orphan block deletion outcome:
* orphaned_blocks_cleaned
* orphaned_block_cleanup_failures
- Add unit tests to test these scenarios with metrics verification:
* Usual path where orphaned blocks are deleted with no error.
* No metrics are updated when orphan block deletion is disabled.
* Induced I/O error causes orphaned blocks lying around that are not
deleted and eventually removed from the set with a action to user.
* Two stages: First induce I/O error, verify metrics shows cleanup
failure count increased. Second remove induced error, verify metrics
remains unchanged.

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/24092/5
--
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: newpatchset
Gerrit-Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
Gerrit-Change-Number: 24092
Gerrit-PatchSet: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 5: Verified-1

Build Failed 

http://jenkins.kudu.apache.org/job/pre_commit/2427/ : FAILURE


-- 
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: 5
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Mar 2026 11:38:36 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 4: Verified-1

Build Failed 

http://jenkins.kudu.apache.org/job/pre_commit/2425/ : FAILURE


-- 
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: 4
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Mar 2026 13:44:18 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

Hello Marton Greber, Alexey Serbin, Zoltan Martonka, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/24092

to look at the new patch set (#4).

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................

[fs] add metrics for untracked orphaned blocks

If orphaned_blocks_ set is cleared after the second flush, to maintain
original behavior and expectation, it may fix the TestEIODuringDelete
failure but it opens the possibility of hitting KUDU-1060 where a number
tombstoned tablets keep the record of all orphaned blocks on persistent
superblock i.e., roll-forward of the block deletions until next restart.
This can severly impact the startup time for a tablet server that has
a lot of tombstoned tablets.

Highlights of the change:
- Add warning logs at CommitDeletedBlocks callers when the commit fails.
- Add per-block level error logs for blocks for which deletion record
could not be committed.
- Add these metrics that can hold the orphan block deletion outcome:
* orphaned_blocks_cleaned
* orphaned_block_cleanup_failures
- Add unit tests to test these scenarios with metrics verification:
* Usual path where orphaned blocks are deleted with no error.
* No metrics are updated when orphan block deletion is disabled.
* Induced I/O error causes orphaned blocks lying around that are not
deleted and eventually removed from the set with a action to user.
* Two stages: First induce I/O error, verify metrics shows cleanup
failure count increased. Second remove induced error, verify metrics
remains unchanged.

git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/24092/4
--
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: newpatchset
Gerrit-Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
Gerrit-Change-Number: 24092
Gerrit-PatchSet: 4
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 2:

(15 comments)

A quick first pass.

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@2528
PS2, Line 2528:   if (PREDICT_FALSE(!s.ok())) {
Why not to move this report/warning piece closer to the source of the non-OK result status, so there is less noise and uncertainty whether block may or may not be left orphaned?  E.g., some where in LogBlockManager::RemoveLogBlocks()?


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@2533
PS2, Line 2533: LOG(WARNING)
There might be many blocks failing to delete if they are on a failed data directory, and this might flood the logs.  I don't think there is a lot of demand for having information on exact block IDs, length, etc. to report right away.  If necessary, it might be found using offline CLI tools.

Also, such detailed block information doesn't provide much value for the suggested `kudu fs check --repair' run since the tool doesn't take any piece of that info as an input.

With that, consider rate/count limiting when outputting this warning to avoid flooding the log with useless details.

As for exact count of orphaned blocks and estimate for the disk space behind, that should be taken care in metrics.  I don't think log is the place to keep track of and account for that.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@2534
PS2, Line 2534: due to failure writing deletion metadata: $3.
I'm not sure this is always due to a failure writing metadata: e.g., what about an attempt to delete blocks in a failed data directory?


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() method instead of spreading this in many places at the upper level?


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-line 'for' cycle scope


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.h
File src/kudu/tablet/tablet_metadata.h:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.h@189
PS2, Line 189:   // Callers may pass a pointer to Flush() or UpdateAndFlush() to receive this information, and
             :   // can then update tablet-level metrics and log the results.
nit: why to mention this here at all when this belongs rather to docs on particular methods/functions?  Such mentions tend to become outdated and confusing on updates on the related methods/functions.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.h@192
PS2, Line 192:     // Number of orphaned blocks successfully deleted from disk.
             :     int blocks_cleaned = 0;
             :     // Number of orphaned blocks that could not be deleted (e.g. I/O error).
             :     // These are retained in the superblock and retried on the next flush.
             :     int blocks_failed = 0;
What's the reason behind using signed integers for counters when using unsigned for 'bytes_failed'?  Is there any reserved/special semantics for negative values for these two fields?


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc@565
PS2, Line 565: (orphaned_blocks_.size() + block_ids.size())
nit: remove the enclosing parentheses?


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc@572
PS2, Line 572: if (blocks.empty()) return stats;
style nit: always use scope braces for 'if', similar to 'for' cycle

Also, make this block evaluated before allocating the 'stats' variable on the stack.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h
File src/kudu/tablet/tablet_metrics.h:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h@87
PS2, Line 87: I/O or hardware errors
nit: does the related code even capable of differentiating between I/O and hardware errors?


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h@88
PS2, Line 88: A sustained non-zero value may indicate disk issues
This is confusing.  Sustained by which means?  These counters are reset to 0 upon restarting tablet servers, aren't they?  Also, they stay non-zero in the lifecycle of a tablet on particular tablet server if any blocks found orphaned during bootstrap.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.h@92
PS2, Line 92: orphaned_block_cleanup_failures_bytes
If introducing this pairing metric for orphaned_block_cleanup_failures, why not to venture introducing similar metric for orphaned_blocks_cleaned?  If somebody in interested in knowing how much space is occupied by orphaned blocks, they might be curious how much was reclaimed to see if the orphaned block cleanup caused drop in disk space utilization since last restart of a tablet server.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc@234
PS2, Line 234: Failed blocks are retried on the next metadata flush.
If this means blocks can be eventually cleaned up during lifecycle of a tablet, then this metric cannot be of 'counter' type, nope.  Switch to the 'gauge' type if the value of this metric isn't monotonically increasing during the lifecycle of a tablet within one continuous run (i.e. no restart) of a tablet server.

Same for the pairing orphaned_block_cleanup_failures_bytes metric below.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc@241
PS2, Line 241:  This is disk space that is tracked in the tablet superblock "
             :                       "but has not been reclaimed. A persistently non-zero value indicates "
             :                       "orphaned data that may be accumulating and causing disk pressure. "
             :                       "Run 'kudu fs check --repair' to manually clean up orphaned blocks.
Move this out and merge into the description of the orphaned_block_cleanup_failures metric.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc@245
PS2, Line 245: kWarn
FWIW, it's already enough to have orphaned_block_cleanup_failures as a metric with kWarn severity to spot an issue and build notifications.  I'd think of moving this metric under the 'kInfo' domain to reduce unnecessary load on metric collection engines which is a well known issue for larg Kudu clusters.



-- 
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: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Mar 2026 20:28:20 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.

Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/24092/7/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/7/src/kudu/fs/log_block_manager.cc@3414
PS7, Line 3414: lbs_by_containers.size());
> Isn't that already in container path (i.e. ToString() output) as the last d
Yes, that makes sense. I think we should leave it as is. I'll take another look tomorrow for any other feedback.



-- 
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: 7
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2026 05:31:19 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.

Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 8: Code-Review+2

Looks good to me, let's wait for earlier reviewers feedback too.


-- 
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: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Apr 2026 23:53:13 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 6:

Build Started http://jenkins.kudu.apache.org/job/pre_commit/2458/


-- 
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: 6
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Apr 2026 06:30:25 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 7:

Build Started http://jenkins.kudu.apache.org/job/pre_commit/2459/


-- 
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: 7
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Apr 2026 06:35:10 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.

Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 7: Code-Review+1

Looks good to me, let's wait for earlier reviewers feedback too.


-- 
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: 7
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Apr 2026 06:42:20 +0000
Gerrit-HasComments: No

[kudu-CR] [WIP] [fs] keep orphaned blocks to cleanup in next flush

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [WIP] [fs] keep orphaned blocks to cleanup in next flush
......................................................................


Patch Set 1: Verified-1

Build Failed 

http://jenkins.kudu.apache.org/job/pre_commit/2364/ : FAILURE


-- 
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: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 12 Mar 2026 13:43:30 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@a3343
PS5, Line 3343: 
> There is no GC'ing logic present in PS5. So, any block for which deletion f
Thanks for the write-up.

The patch that implements GC isn't in the same review stack with this patch, even it seems logically and textually dependent on this one.  This is what confused me, so I assumed that's all done in this one patch.


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3350
PS5, Line 3350:   for (auto& lb : lbs) {
              :     auto& lbs_by_container = LookupOrInsert(&lbs_by_containers, lb->container(), {});
              :     lbs_by_container.emplace_back(std::move(lb));
> This is not different from what is being tracked under orphaned_block_clean
I see: LogBlockDeletionTransaction::CommitDeletedBlocks(), being a part of TabletMetadata::DeleteOrphanedBlocks(), invokes LogBlockManager::RemoveLogBlocks().  Now this makes sense to me.

And here are logging this counter/info as 'orphaned_count' with KLOG_EVERY_N_SECS as of PS7.

Thanks for the clarification.


http://gerrit.cloudera.org:8080/#/c/24092/5/src/kudu/fs/log_block_manager.cc@3417
PS5, Line 3417:   return first_failure;
> In any case, this needs to change because src/kudu/util/logging.h (included
Thanks for the explanation.  I guess it means you consider this as related to the patch as-is.  Feel free to keep it in then.  However, you could switch from deprecated LOG_EVERY_N to KLOG_EVERY_N and similar in a separate clean-up patch, and then base this patch on top of that one in gerrit.  That's how it usually done, and it's much cleaner way do deal with such circus in most cases.


http://gerrit.cloudera.org:8080/#/c/24092/7/src/kudu/util/logging.h
File src/kudu/util/logging.h:

PS7: 
Please don't avoid piling up logically non-related changes in a single changelist.


Please post this update as a separate changelist and re-base this 24092 patch on top.

Thank you!



-- 
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: 7
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Apr 2026 08:27:30 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 8: Verified+1

Build Successful 

http://jenkins.kudu.apache.org/job/pre_commit/2513/ : SUCCESS


-- 
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: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Apr 2026 14:58:58 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................

[fs] add metrics for untracked orphaned blocks

Highlights of the change:
- Add warning logs at CommitDeletedBlocks callers when the commit fails.
- Add per-block level error logs for blocks for which deletion record
could not be committed.
- Add these metrics that can hold the orphan block deletion outcome:
* orphaned_blocks_cleaned
* orphaned_block_cleanup_failures
- Add unit tests to test these scenarios with metrics verification:
* Usual path where orphaned blocks are deleted with no error.
* No metrics are updated when orphan block deletion is disabled.
* Induced I/O error causes orphaned blocks lying around that are not
deleted and eventually removed from the set with a action to user.
* Two stages: First induce I/O error, verify metrics shows cleanup
failure count increased. Second remove induced error, verify metrics
remains unchanged.

Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
Reviewed-on: http://gerrit.cloudera.org:8080/24092
Tested-by: Kudu Jenkins
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/fs/log_block_manager.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_metadata.h
M src/kudu/tablet/tablet_metrics.cc
M src/kudu/tablet/tablet_metrics.h
8 files changed, 362 insertions(+), 29 deletions(-)

Approvals:
Kudu Jenkins: Verified
Abhishek Chennaka: Looks good to me, approved
Alexey Serbin: Looks good to me, approved

--
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: merged
Gerrit-Change-Id: Id386d9fc8d0900839e229e66772f35299b3ef2e9
Gerrit-Change-Number: 24092
Gerrit-PatchSet: 9
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24092 )

Change subject: [fs] add metrics for untracked orphaned blocks
......................................................................


Patch Set 8: Code-Review+2

Thank you very much for putting together this patch and addressing all the feedback!

PS8 looks good to me.


-- 
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: 8
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Apr 2026 07:25:55 +0000
Gerrit-HasComments: No

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

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 2:

(1 comment)

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);
> That would involve passing TabletMetrics* to TabletMetadata so that callee 
Let me see if there is a better way to address this.



-- 
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: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Mar 2026 05:54:32 +0000
Gerrit-HasComments: Yes

[kudu-CR] [fs] add metrics for untracked orphaned blocks

Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.

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 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@2529
PS2, Line 2529: help
> nit: held ?
Done


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@2530
PS2, Line 2530: lyring
> nit: lying ?
Done


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/fs/log_block_manager.cc@3363
PS2, Line 3363:       // RemoveBlockIdsFromMetadata writes records in the same order as clbs,
> Does this assumption hold for LogBlockContainerRdbMeta?
Yes, the assumption should hold for RockDB variant as well, given the logic in there is similar to native container.

However, I found one bug in RocksDB function that could result in it send back incorrect deleted_block_ids.size() if there was some intermediate error in writing to persistent storage that resulted in partial deletion.
It seems to be missing cleanup routine that does a resize of out parameter if it runs into an error i.e., a partial deletion case scenario.

I have raised a separate gerrit patch to address this issue: https://gerrit.cloudera.org/#/c/24134/

Please take a look.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata-test.cc
File src/kudu/tablet/tablet_metadata-test.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata-test.cc@441
PS2, Line 441: TEST_F(TestTabletMetadata, TestOrphanBlockDeletionFailed) {
> Can we also assert orphaned_block_cleanup_failures_bytes in this test?
I wanted to do that as well but the problem with that is the way we fetch size of block. The block size is determined by calling OpenBlock on each block for which delete failed. OpenBlock looks up into a in-memory shard of blocks (managed_block_shards_) indexed with block_id. Since we have already erased block entry from managed_block_shards_ before hitting EIO error which happens at later stage, OpenBlock fails and we end up with 0 bytes.

Come to think of it, it seems to be miss from my side because orphaned_block_cleanup_failures_bytes is always going to be 0 because removal of block from in-memory shard of blocks happens way before actual on-disk deletion attempt is made. It is incorrect to rely on OpenBlock to get size for those blocks for which delete failed.

Anyway, this is a cosmetic change to get more insight into bytes and overhead of making it work doesn't seem cost-effective. I am inclined toward removing this metric altogether.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc@601
PS2, Line 601:     auto bm_size = fs_manager()->block_manager();
> do we need this declaration here? can't we reuse the above declared bm?
We can reuse. Since, I have removed the use of failed bytes stat altogether so this code block won't be there now.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metadata.cc@623
PS2, Line 623:   // Regardless of whether we deleted all the blocks or not, remove them from
             :   // the orphaned blocks list. If we failed to delete the blocks due to
             :   // hardware issues, there's not much we can do and we assume the disk isn't
             :   // coming back. At worst, this leaves some untracked orphaned blocks.
> Maybe I dont have the complete picture yet, but doesnt this contradict that
You are right. I missed out to make correction in above LOG message in PS2. In PS1, I had tried to incorporate the full fix by keeping track of un-deleted blocks in the superblock but it turned out to be a bit more complex. I chose to keep that change separate: https://gerrit.cloudera.org/#/c/24125/

Since this patch only takes care of collecting metrics, LOG_WITH_PREFIX comment is no more applicable.
Removed.


http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc
File src/kudu/tablet/tablet_metrics.cc:

http://gerrit.cloudera.org:8080/#/c/24092/2/src/kudu/tablet/tablet_metrics.cc@247
PS2, Line 247: 
> nit: extra new line?
Done



-- 
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: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Mar 2026 17:08:40 +0000
Gerrit-HasComments: Yes