Add validate flag to sft load api

Prathyusha Garre · Prathyusha Garre · commit 504506dd676a · 2024-07-25T17:44:32.000+05:30
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobUtils.java
@@ -395,6 +395,10 @@ public static Path getMobTableDir(Path rootDir, TableName tableName) {
     return CommonFSUtils.getTableDir(getMobHome(rootDir), tableName);
   }
 
+  public static Path getMobTableDir(Configuration conf, TableName tableName) {
+    return getMobTableDir(new Path(conf.get(HConstants.HBASE_DIR)), tableName);
+  }
+
   /**
    * Gets the region dir of the mob files. It's
    * {HBASE_DIR}/mobdir/data/{namespace}/{tableName}/{regionEncodedName}.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/DefaultStoreFileTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/DefaultStoreFileTracker.java
@@ -64,8 +64,9 @@ protected void doAddCompactionResults(Collection<StoreFileInfo> compactedFiles,
   }
 
   @Override
-  protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly) throws IOException {
-    List<StoreFileInfo> files = getStoreFiles(ctx.getFamily().getNameAsString(), true);
+  protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly, boolean validate)
+    throws IOException {
+    List<StoreFileInfo> files = getStoreFiles(ctx.getFamily().getNameAsString(), validate);
     return files != null ? files : Collections.emptyList();
   }
 
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java
@@ -67,7 +67,8 @@ public FileBasedStoreFileTracker(Configuration conf, boolean isPrimaryReplica, S
   }
 
   @Override
-  protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly) throws IOException {
+  protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly, boolean validate)
+    throws IOException {
     StoreFileList list = backedFile.load(readOnly);
     if (list == null) {
       return Collections.emptyList();
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java
@@ -61,8 +61,9 @@ public boolean requireWritingToTmpDirFirst() {
   }
 
   @Override
-  protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly) throws IOException {
-    List<StoreFileInfo> files = src.doLoadStoreFiles(readOnly);
+  protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly, boolean validate)
+    throws IOException {
+    List<StoreFileInfo> files = src.doLoadStoreFiles(readOnly, validate);
     if (!readOnly) {
       dst.doSetStoreFiles(files);
     }
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java
@@ -57,6 +57,11 @@ public interface StoreFileTracker {
    */
   List<StoreFileInfo> load() throws IOException;
 
+  /**
+   * Load the store files list when opening a region.
+   */
+  List<StoreFileInfo> load(boolean validate) throws IOException;
+
   /**
    * Add new store files.
    * <p/>
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java
@@ -80,7 +80,12 @@ protected StoreFileTrackerBase(Configuration conf, boolean isPrimaryReplica, Sto
 
   @Override
   public final List<StoreFileInfo> load() throws IOException {
-    return doLoadStoreFiles(!isPrimaryReplica);
+    return load(true);
+  }
+
+  @Override
+  public final List<StoreFileInfo> load(boolean validate) throws IOException {
+    return doLoadStoreFiles(!isPrimaryReplica, validate);
   }
 
   @Override
@@ -323,7 +328,8 @@ public StoreFileInfo getStoreFileInfo(FileStatus fileStatus, Path initialPath,
    * are allowed to do the cleanup work. For secondary replicas, we will set {@code readOnly} to
    * {@code true}.
    */
-  protected abstract List<StoreFileInfo> doLoadStoreFiles(boolean readOnly) throws IOException;
+  protected abstract List<StoreFileInfo> doLoadStoreFiles(boolean readOnly, boolean validate)
+    throws IOException;
 
   protected abstract void doAddNewStoreFiles(Collection<StoreFileInfo> newFiles) throws IOException;
 
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotManifestV1.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotManifestV1.java
@@ -185,7 +185,7 @@ static SnapshotRegionManifest buildManifestFromDisk(final Configuration conf, fi
       for (String familyName : familyNames) {
         StoreFileTracker sft = StoreFileTrackerFactory.create(conf, htd,
           htd.getColumnFamily(familyName.getBytes()), regionFs, false);
-        List<StoreFileInfo> storeFiles = sft.load();
+        List<StoreFileInfo> storeFiles = sft.load(false);
         if (storeFiles == null) {
           LOG.debug("No files under family: " + familyName);
           continue;
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobStoreCompaction.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobStoreCompaction.java
@@ -60,6 +60,7 @@
 import org.apache.hadoop.hbase.io.hfile.HFileContextBuilder;
 import org.apache.hadoop.hbase.regionserver.BloomType;
 import org.apache.hadoop.hbase.regionserver.HRegion;
+import org.apache.hadoop.hbase.regionserver.HRegionFileSystem;
 import org.apache.hadoop.hbase.regionserver.HStore;
 import org.apache.hadoop.hbase.regionserver.HStoreFile;
 import org.apache.hadoop.hbase.regionserver.InternalScanner;
@@ -331,9 +332,13 @@ private long countMobCellsInMetadata() throws IOException {
     CacheConfig cacheConfig = new CacheConfig(copyOfConf);
     if (fs.exists(mobDirPath)) {
       // TODO: use sft.load() api here
-      StoreFileTracker sft =
-        StoreFileTrackerFactory.create(copyOfConf, true, StoreContext.getBuilder()
-          .withColumnFamilyDescriptor(familyDescriptor).withCacheConfig(cacheConfig).build());
+      HRegionFileSystem regionFs = HRegionFileSystem.create(copyOfConf, fs,
+        MobUtils.getMobTableDir(copyOfConf, tableDescriptor.getTableName()),
+        region.getRegionInfo());
+      StoreFileTracker sft = StoreFileTrackerFactory.create(copyOfConf, true,
+        StoreContext.getBuilder().withColumnFamilyDescriptor(familyDescriptor)
+          .withFamilyStoreDirectoryPath(mobDirPath).withCacheConfig(cacheConfig)
+          .withRegionFileSystem(regionFs).build());
       FileStatus[] files = UTIL.getTestFileSystem().listStatus(mobDirPath);
       for (FileStatus file : files) {
         HStoreFile sf =
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStoreFile.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStoreFile.java
@@ -351,7 +351,7 @@ public void testHFileLink() throws IOException {
     // this should be the SFT for the destination link file path, though it is not
     // being used right now, for the next patch file link creation logic also would
     // move to SFT interface.
-    StoreFileTracker sft = StoreFileTrackerFactory.create(conf, true,
+    StoreFileTracker sft = StoreFileTrackerFactory.create(testConf, true,
       StoreContext.getBuilder()
         .withFamilyStoreDirectoryPath(new Path(dstHri.getRegionNameAsString(), TEST_FAMILY))
         .withRegionFileSystem(dstRegionFs).build());
@@ -416,20 +416,20 @@ public void testReferenceToHFileLink() throws IOException {
     RegionInfo splitHriB =
       RegionInfoBuilder.newBuilder(hri.getTable()).setStartKey(SPLITKEY).build();
 
-    StoreFileTracker sft = StoreFileTrackerFactory.create(conf, true,
+    StoreFileTracker sft = StoreFileTrackerFactory.create(testConf, true,
       StoreContext.getBuilder()
         .withFamilyStoreDirectoryPath(new Path(hriClone.getRegionNameAsString(), TEST_FAMILY))
         .withRegionFileSystem(cloneRegionFs).build());
 
     HRegionFileSystem splitRegionAFs = HRegionFileSystem.createRegionOnFileSystem(testConf, fs,
       CommonFSUtils.getTableDir(testDir, splitHriA.getTable()), splitHriA);
-    StoreFileTracker sftA = StoreFileTrackerFactory.create(conf, true,
+    StoreFileTracker sftA = StoreFileTrackerFactory.create(testConf, true,
       StoreContext.getBuilder()
         .withFamilyStoreDirectoryPath(new Path(splitHriA.getRegionNameAsString(), TEST_FAMILY))
         .withRegionFileSystem(splitRegionAFs).build());
     HRegionFileSystem splitRegionBFs = HRegionFileSystem.createRegionOnFileSystem(testConf, fs,
       CommonFSUtils.getTableDir(testDir, splitHriB.getTable()), splitHriB);
-    StoreFileTracker sftB = StoreFileTrackerFactory.create(conf, true,
+    StoreFileTracker sftB = StoreFileTrackerFactory.create(testConf, true,
       StoreContext.getBuilder()
         .withFamilyStoreDirectoryPath(new Path(splitHriB.getRegionNameAsString(), TEST_FAMILY))
         .withRegionFileSystem(splitRegionBFs).build());
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMergesSplitsAddToTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMergesSplitsAddToTracker.java
@@ -221,8 +221,10 @@ private Pair<StoreFileInfo, String> copyFileInTheStoreDir(HRegion region) throws
     Path storeDir = region.getRegionFileSystem().getStoreDir("info");
     // gets the single file
     HRegionFileSystem regionFs = region.getRegionFileSystem();
-    StoreFileTracker sft = StoreFileTrackerFactory.create(region.getBaseConf(), true, StoreContext
-      .getBuilder().withFamilyStoreDirectoryPath(storeDir).withRegionFileSystem(regionFs).build());
+    StoreFileTracker sft = StoreFileTrackerFactory.create(region.getBaseConf(), true,
+      StoreContext.getBuilder().withFamilyStoreDirectoryPath(storeDir)
+        .withColumnFamilyDescriptor(ColumnFamilyDescriptorBuilder.of(FAMILY_NAME))
+        .withRegionFileSystem(regionFs).build());
     List<StoreFileInfo> infos = sft.load();
     StoreFileInfo fileInfo = infos.get(0);
     // make a copy of the valid file staight into the store dir, so that it's not tracked.
@@ -237,9 +239,10 @@ private void validateDaughterRegionsFiles(HRegion region, String originalFileNam
     String untrackedFile) throws IOException {
     // verify there's no link for the untracked, copied file in first region
     HRegionFileSystem regionFs = region.getRegionFileSystem();
-    StoreFileTracker sft = StoreFileTrackerFactory.create(region.getBaseConf(), true,
+    StoreFileTracker sft = StoreFileTrackerFactory.create(regionFs.getFileSystem().getConf(), true,
       StoreContext.getBuilder()
         .withFamilyStoreDirectoryPath(new Path(regionFs.getRegionDir(), "info"))
+        .withColumnFamilyDescriptor(ColumnFamilyDescriptorBuilder.of(FAMILY_NAME))
         .withRegionFileSystem(regionFs).build());
     List<StoreFileInfo> infos = sft.load();
     assertThat(infos, everyItem(hasProperty("activeFileName", not(containsString(untrackedFile)))));
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerForTest.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerForTest.java
@@ -58,7 +58,8 @@ protected void doAddNewStoreFiles(Collection<StoreFileInfo> newFiles) throws IOE
   }
 
   @Override
-  protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly) throws IOException {
+  protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly, boolean validate)
+    throws IOException {
     return new ArrayList<>(trackedFiles.get(storeId));
   }
 

Original file line number	Diff line number	Diff line change
`@@ -64,8 +64,9 @@ protected void doAddCompactionResults(Collection<StoreFileInfo> compactedFiles,`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`@Override`
`67`		`- protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly) throws IOException {`
`68`		`- List<StoreFileInfo> files = getStoreFiles(ctx.getFamily().getNameAsString(), true);`
	`67`	`+ protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly, boolean validate)`
	`68`	`+ throws IOException {`
	`69`	`+ List<StoreFileInfo> files = getStoreFiles(ctx.getFamily().getNameAsString(), validate);`
`69`	`70`	`return files != null ? files : Collections.emptyList();`
`70`	`71`	`}`
`71`	`72`
Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,8 @@ public FileBasedStoreFileTracker(Configuration conf, boolean isPrimaryReplica, S`
`67`	`67`	`}`
`68`	`68`
`69`	`69`	`@Override`
`70`		`- protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly) throws IOException {`
	`70`	`+ protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly, boolean validate)`
	`71`	`+ throws IOException {`
`71`	`72`	`StoreFileList list = backedFile.load(readOnly);`
`72`	`73`	`if (list == null) {`
`73`	`74`	`return Collections.emptyList();`
Original file line number	Diff line number	Diff line change
`@@ -61,8 +61,9 @@ public boolean requireWritingToTmpDirFirst() {`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`@Override`
`64`		`- protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly) throws IOException {`
`65`		`- List<StoreFileInfo> files = src.doLoadStoreFiles(readOnly);`
	`64`	`+ protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly, boolean validate)`
	`65`	`+ throws IOException {`
	`66`	`+ List<StoreFileInfo> files = src.doLoadStoreFiles(readOnly, validate);`
`66`	`67`	`if (!readOnly) {`
`67`	`68`	`dst.doSetStoreFiles(files);`
`68`	`69`	`}`