(hbase) branch branch-3 updated: [ADDENDUM] HBASE-28884 SFT's BrokenStoreFileCleaner may cause data loss (#6326)

[wc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wchevreuil pushed a commit to branch branch-3
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-3 by this push:
     new d603aa61c52 [ADDENDUM] HBASE-28884 SFT's BrokenStoreFileCleaner may cause data loss (#6326)
d603aa61c52 is described below

commit d603aa61c5293a5de939c9c5ad59e61dcccadde3
Author: Wellington Ramos Chevreuil <wc...@apache.org>
AuthorDate: Tue Oct 1 20:40:30 2024 +0100

    [ADDENDUM] HBASE-28884 SFT's BrokenStoreFileCleaner may cause data loss (#6326)
    
    Signed-off-by: Duo Zhang <zh...@apache.org>
---
 .../apache/hadoop/hbase/regionserver/BrokenStoreFileCleaner.java  | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/BrokenStoreFileCleaner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/BrokenStoreFileCleaner.java
index df71adecd0c..c5151551587 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/BrokenStoreFileCleaner.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/BrokenStoreFileCleaner.java
@@ -147,6 +147,14 @@ public class BrokenStoreFileCleaner extends ScheduledChore {
       return;
     }
 
+    // Adding the check here is enough to prevent accidentally deleting of intact HFiles.
+    // If the region is still available, i.e, not closed or closing, all above checks are valid
+    // so we are safe to delete the 'broken' files. And if the region is closed or closing when we
+    // reach here, since we will always give up deleting, the only possible inconsistency is that
+    // the file is broken since the region is not closing or close when performing above checks
+    // but here we give up deleting it. So in either way, we will not accidentally deleting intact
+    // HFiles, but only give up deleting broken files. But it is OK since we can still
+    // delete them in the next round of cleaner run.
     if (!store.getHRegion().isAvailable()) {
       LOG.trace("This store's region {} is no longer open, so it might have moved elsewhere. "
         + "Skipping cleanup.", store.getRegionInfo().getEncodedName());