This is an automated email from the ASF dual-hosted git repository.
apurtell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new 8e25d756697 HBASE-29458 SFT removeStoreFiles api to only archive
physical files and ignore virtual links (#7168)
8e25d756697 is described below
commit 8e25d756697003f16225b78f4b6d8355a8257b12
Author: gvprathyusha6 <[email protected]>
AuthorDate: Wed Aug 13 05:24:18 2025 +0530
HBASE-29458 SFT removeStoreFiles api to only archive physical files and
ignore virtual links (#7168)
Signed-off-by: Andrew Purtell <[email protected]>
---
.../hbase/regionserver/HRegionFileSystem.java | 23 -------------
.../apache/hadoop/hbase/regionserver/HStore.java | 5 +--
.../hadoop/hbase/regionserver/StoreEngine.java | 3 +-
.../MigrationStoreFileTracker.java | 6 ++++
.../storefiletracker/StoreFileTracker.java | 7 ++++
.../storefiletracker/StoreFileTrackerBase.java | 12 +++++++
.../TestCompactionArchiveConcurrentClose.java | 38 +++++++++++++---------
.../hadoop/hbase/regionserver/TestHRegion.java | 3 +-
.../hadoop/hbase/regionserver/TestHStore.java | 6 ++--
9 files changed, 58 insertions(+), 45 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
index 827051b9b3e..e5172527122 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
@@ -440,29 +440,6 @@ public class HRegionFileSystem {
return dstPath;
}
- /**
- * Archives the specified store file from the specified family.
- * @param familyName Family that contains the store files
- * @param filePath {@link Path} to the store file to remove
- * @throws IOException if the archiving fails
- */
- public void removeStoreFile(final String familyName, final Path filePath)
throws IOException {
- HFileArchiver.archiveStoreFile(this.conf, this.fs, this.regionInfoForFs,
this.tableDir,
- Bytes.toBytes(familyName), filePath);
- }
-
- /**
- * Closes and archives the specified store files from the specified family.
- * @param familyName Family that contains the store files
- * @param storeFiles set of store files to remove
- * @throws IOException if the archiving fails
- */
- public void removeStoreFiles(String familyName, Collection<HStoreFile>
storeFiles)
- throws IOException {
- HFileArchiver.archiveStoreFiles(this.conf, this.fs, this.regionInfoForFs,
this.tableDir,
- Bytes.toBytes(familyName), storeFiles);
- }
-
/**
* Bulk load: Add a specified store file to the specified family. If the
source file is on the
* same different file-system is moved from the source location to the
destination location,
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
index bb7f73d9ed7..98299c47302 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
@@ -2328,8 +2328,9 @@ public class HStore
LOG.debug("Moving the files {} to archive", filesToRemove);
// Only if this is successful it has to be removed
try {
-
getRegionFileSystem().removeStoreFiles(this.getColumnFamilyDescriptor().getNameAsString(),
- filesToRemove);
+ StoreFileTracker storeFileTracker =
+ StoreFileTrackerFactory.create(conf, isPrimaryReplicaStore(),
storeContext);
+ storeFileTracker.removeStoreFiles(filesToRemove);
} catch (FailedArchiveException fae) {
// Even if archiving some files failed, we still need to clear out
any of the
// files which were successfully archived. Otherwise we will receive
a
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java
index 4380c5cfc0b..30cf5e2a92f 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreEngine.java
@@ -351,8 +351,7 @@ public abstract class StoreEngine<SF extends StoreFlusher,
CP extends Compaction
results.removeAll(filesToRemove);
if (!filesToRemove.isEmpty() && ctx.isPrimaryReplicaStore()) {
LOG.debug("Moving the files {} to archive", filesToRemove);
-
ctx.getRegionFileSystem().removeStoreFiles(ctx.getFamily().getNameAsString(),
- filesToRemove);
+ storeFileTracker.removeStoreFiles(filesToRemove);
}
}
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
index c4962885a07..99aaa20c6ff 100644
---
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
@@ -21,6 +21,7 @@ import java.io.IOException;
import java.util.Collection;
import java.util.List;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.regionserver.HStoreFile;
import org.apache.hadoop.hbase.regionserver.StoreContext;
import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
import org.apache.yetus.audience.InterfaceAudience;
@@ -88,6 +89,11 @@ class MigrationStoreFileTracker extends StoreFileTrackerBase
{
"Should not call this method on " + getClass().getSimpleName());
}
+ @Override
+ public void removeStoreFiles(List<HStoreFile> storeFiles) throws IOException
{
+ dst.removeStoreFiles(storeFiles);
+ }
+
static Class<? extends StoreFileTracker> getSrcTrackerClass(Configuration
conf) {
return StoreFileTrackerFactory.getStoreFileTrackerClassForMigration(conf,
SRC_IMPL);
}
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
index 7023ff5115a..595d5f4d1fc 100644
---
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
@@ -26,6 +26,7 @@ import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.io.Reference;
import org.apache.hadoop.hbase.regionserver.CreateStoreFileWriterParams;
+import org.apache.hadoop.hbase.regionserver.HStoreFile;
import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
import org.apache.hadoop.hbase.regionserver.StoreFileWriter;
import org.apache.yetus.audience.InterfaceAudience;
@@ -146,4 +147,10 @@ public interface StoreFileTracker {
String createFromHFileLink(final String hfileName, final boolean
createBackRef)
throws IOException;
+ /**
+ * Closes and archives the specified store files from the specified family.
+ * @param storeFiles set of store files to remove
+ * @throws IOException if the archiving fails
+ */
+ void removeStoreFiles(List<HStoreFile> storeFiles) throws IOException;
}
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
index 33b294ac89b..779a114af59 100644
---
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
@@ -33,6 +33,7 @@ import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.backup.HFileArchiver;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.io.HFileLink;
@@ -44,6 +45,7 @@ import org.apache.hadoop.hbase.io.hfile.HFile;
import org.apache.hadoop.hbase.io.hfile.HFileContext;
import org.apache.hadoop.hbase.io.hfile.HFileContextBuilder;
import org.apache.hadoop.hbase.regionserver.CreateStoreFileWriterParams;
+import org.apache.hadoop.hbase.regionserver.HStoreFile;
import org.apache.hadoop.hbase.regionserver.StoreContext;
import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
import org.apache.hadoop.hbase.regionserver.StoreFileWriter;
@@ -373,6 +375,16 @@ abstract class StoreFileTrackerBase implements
StoreFileTracker {
createBackRef);
}
+ public void removeStoreFiles(List<HStoreFile> storeFiles) throws IOException
{
+ archiveStoreFiles(storeFiles);
+ }
+
+ protected void archiveStoreFiles(List<HStoreFile> storeFiles) throws
IOException {
+ HFileArchiver.archiveStoreFiles(this.conf,
ctx.getRegionFileSystem().getFileSystem(),
+ ctx.getRegionInfo(), ctx.getRegionFileSystem().getTableDir(),
ctx.getFamily().getName(),
+ storeFiles);
+ }
+
/**
* For primary replica, we will call load once when opening a region, and
the implementation could
* choose to do some cleanup work. So here we use {@code readOnly} to
indicate that whether you
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionArchiveConcurrentClose.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionArchiveConcurrentClose.java
index 8c5770d8499..a6e565cdb41 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionArchiveConcurrentClose.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionArchiveConcurrentClose.java
@@ -29,7 +29,6 @@ import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtil;
@@ -41,6 +40,8 @@ import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.RegionInfoBuilder;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import
org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory;
+import
org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerForTest;
import org.apache.hadoop.hbase.testclassification.RegionServerTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.util.Bytes;
@@ -71,6 +72,9 @@ public class TestCompactionArchiveConcurrentClose {
private Path testDir;
private AtomicBoolean archived = new AtomicBoolean();
+ // Static field to track archived state for the static inner class
+ private static final AtomicBoolean STATIC_ARCHIVED = new AtomicBoolean();
+
@Rule
public TestName name = new TestName();
@@ -79,6 +83,11 @@ public class TestCompactionArchiveConcurrentClose {
testUtil = new HBaseTestingUtil();
testDir = testUtil.getDataTestDir("TestStoreFileRefresherChore");
CommonFSUtils.setRootDir(testUtil.getConfiguration(), testDir);
+ // Configure the test to use our custom WaitingStoreFileTracker
+ testUtil.getConfiguration().set(StoreFileTrackerFactory.TRACKER_IMPL,
+ WaitingStoreFileTracker.class.getName());
+ // Reset the static archived flag
+ STATIC_ARCHIVED.set(false);
}
@After
@@ -127,6 +136,7 @@ public class TestCompactionArchiveConcurrentClose {
for (HStoreFile file : storefiles) {
assertFalse(file.isCompactedAway());
}
+ System.out.println("Finished compaction ");
// Do compaction
region.compact(true);
@@ -139,9 +149,9 @@ public class TestCompactionArchiveConcurrentClose {
};
cleanerThread.start();
// wait for cleaner to pause
- synchronized (archived) {
- if (!archived.get()) {
- archived.wait();
+ synchronized (STATIC_ARCHIVED) {
+ if (!STATIC_ARCHIVED.get()) {
+ STATIC_ARCHIVED.wait();
}
}
final AtomicReference<Exception> closeException = new AtomicReference<>();
@@ -171,7 +181,7 @@ public class TestCompactionArchiveConcurrentClose {
Path tableDir = CommonFSUtils.getTableDir(testDir, htd.getTableName());
HRegionFileSystem fs =
- new WaitingHRegionFileSystem(conf, tableDir.getFileSystem(conf),
tableDir, info);
+ new HRegionFileSystem(conf, tableDir.getFileSystem(conf), tableDir,
info);
ChunkCreator.initialize(MemStoreLAB.CHUNK_SIZE_DEFAULT, false, 0, 0, 0,
null,
MemStoreLAB.INDEX_CHUNK_SIZE_PERCENTAGE_DEFAULT);
final Configuration walConf = new Configuration(conf);
@@ -184,20 +194,18 @@ public class TestCompactionArchiveConcurrentClose {
return region;
}
- private class WaitingHRegionFileSystem extends HRegionFileSystem {
+ public static class WaitingStoreFileTracker extends StoreFileTrackerForTest {
- public WaitingHRegionFileSystem(final Configuration conf, final FileSystem
fs,
- final Path tableDir, final RegionInfo regionInfo) {
- super(conf, fs, tableDir, regionInfo);
+ public WaitingStoreFileTracker(Configuration conf, boolean
isPrimaryReplica, StoreContext ctx) {
+ super(conf, isPrimaryReplica, ctx);
}
@Override
- public void removeStoreFiles(String familyName, Collection<HStoreFile>
storeFiles)
- throws IOException {
- super.removeStoreFiles(familyName, storeFiles);
- archived.set(true);
- synchronized (archived) {
- archived.notifyAll();
+ public void removeStoreFiles(List<HStoreFile> storeFiles) throws
IOException {
+ super.removeStoreFiles(storeFiles);
+ STATIC_ARCHIVED.set(true);
+ synchronized (STATIC_ARCHIVED) {
+ STATIC_ARCHIVED.notifyAll();
}
try {
// unfortunately we can't use a stronger barrier here as the fix
synchronizing
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
index 661d9f6c9e4..da1c11ba64c 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
@@ -66,6 +66,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FSDataOutputStream;
@@ -5710,7 +5711,6 @@ public class TestHRegion {
// move the file of the primary region to the archive, simulating a
compaction
Collection<HStoreFile> storeFiles =
primaryRegion.getStore(families[0]).getStorefiles();
-
primaryRegion.getRegionFileSystem().removeStoreFiles(Bytes.toString(families[0]),
storeFiles);
HRegionFileSystem regionFs = primaryRegion.getRegionFileSystem();
StoreFileTracker sft =
StoreFileTrackerFactory.create(primaryRegion.getBaseConf(), false,
StoreContext.getBuilder()
@@ -5718,6 +5718,7 @@ public class TestHRegion {
.withFamilyStoreDirectoryPath(
new Path(regionFs.getRegionDir(), Bytes.toString(families[0])))
.withRegionFileSystem(regionFs).build());
+ sft.removeStoreFiles(storeFiles.stream().collect(Collectors.toList()));
Collection<StoreFileInfo> storeFileInfos = sft.load();
Assert.assertTrue(storeFileInfos == null || storeFileInfos.isEmpty());
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java
index 92c288539bc..179297bd873 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHStore.java
@@ -1033,8 +1033,10 @@ public class TestHStore {
for (int i = 0; i <= index; i++) {
sf = it.next();
}
- store.getRegionFileSystem().removeStoreFiles(store.getColumnFamilyName(),
- Lists.newArrayList(sf));
+ StoreFileTracker sft =
+
StoreFileTrackerFactory.create(store.getRegionFileSystem().getFileSystem().getConf(),
+ store.isPrimaryReplicaStore(), store.getStoreContext());
+ sft.removeStoreFiles(Lists.newArrayList(sf));
}
private void closeCompactedFile(int index) throws IOException {
