From 7e6c7f337e811a92ed31665cffd312826f6c5fc8 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Thu, 6 Jan 2022 21:09:26 +0800 Subject: [PATCH 1/2] HBASE-26639 The implementation of TestMergesSplitsAddToTracker is problematic --- .../TestMergesSplitsAddToTracker.java | 47 +++++++++++-------- .../TestStoreFileTracker.java | 25 +++++++--- 2 files changed, 45 insertions(+), 27 deletions(-) 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 index 68fc444493c4..588641a6209d 100644 --- 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 @@ -24,11 +24,9 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.UUID; import java.util.concurrent.TimeUnit; - import org.apache.commons.lang3.mutable.MutableBoolean; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; @@ -37,10 +35,14 @@ import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtil; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.TableNameTestRule; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionInfoBuilder; import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.regionserver.storefiletracker.TestStoreFileTracker; import org.apache.hadoop.hbase.testclassification.LargeTests; @@ -55,7 +57,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; -import org.junit.rules.TestName; @Category({RegionServerTests.class, LargeTests.class}) @@ -66,15 +67,16 @@ public class TestMergesSplitsAddToTracker { HBaseClassTestRule.forClass(TestMergesSplitsAddToTracker.class); private static HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil(); + + private static final String FAMILY_NAME_STR = "info"; - public static final byte[] FAMILY_NAME = Bytes.toBytes("info"); + private static final byte[] FAMILY_NAME = Bytes.toBytes(FAMILY_NAME_STR); @Rule - public TestName name = new TestName(); + public TableNameTestRule name = new TableNameTestRule(); @BeforeClass public static void setupClass() throws Exception { - TEST_UTIL.getConfiguration().set(TRACKER_IMPL, TestStoreFileTracker.class.getName()); TEST_UTIL.startMiniCluster(); } @@ -85,13 +87,24 @@ public static void afterClass() throws Exception { @Before public void setup(){ - TestStoreFileTracker.trackedFiles = new HashMap<>(); + TestStoreFileTracker.clear(); + } + + private TableName createTable(byte[] splitKey) throws IOException { + TableDescriptor td = TableDescriptorBuilder.newBuilder(name.getTableName()) + .setColumnFamily(ColumnFamilyDescriptorBuilder.of(FAMILY_NAME)) + .setValue(TRACKER_IMPL, TestStoreFileTracker.class.getName()).build(); + if (splitKey != null) { + TEST_UTIL.getAdmin().createTable(td, new byte[][] { splitKey }); + } else { + TEST_UTIL.getAdmin().createTable(td); + } + return td.getTableName(); } @Test public void testCommitDaughterRegion() throws Exception { - TableName table = TableName.valueOf(name.getMethodName()); - TEST_UTIL.createTable(table, FAMILY_NAME); + TableName table = createTable(null); //first put some data in order to have a store file created putThreeRowsAndFlush(table); HRegion region = TEST_UTIL.getHBaseCluster().getRegions(table).get(0); @@ -125,8 +138,7 @@ public void testCommitDaughterRegion() throws Exception { @Test public void testCommitMergedRegion() throws Exception { - TableName table = TableName.valueOf(name.getMethodName()); - TEST_UTIL.createTable(table, FAMILY_NAME); + TableName table = createTable(null); //splitting the table first TEST_UTIL.getAdmin().split(table, Bytes.toBytes("002")); //Add data and flush to create files in the two different regions @@ -163,8 +175,7 @@ public void testCommitMergedRegion() throws Exception { @Test public void testSplitLoadsFromTracker() throws Exception { - TableName table = TableName.valueOf(name.getMethodName()); - TEST_UTIL.createTable(table, FAMILY_NAME); + TableName table = createTable(null); //Add data and flush to create files in the two different regions putThreeRowsAndFlush(table); HRegion region = TEST_UTIL.getHBaseCluster().getRegions(table).get(0); @@ -182,9 +193,7 @@ public void testSplitLoadsFromTracker() throws Exception { @Test public void testMergeLoadsFromTracker() throws Exception { - TableName table = TableName.valueOf(name.getMethodName()); - TEST_UTIL.createTable(table, new byte[][]{FAMILY_NAME}, - new byte[][]{Bytes.toBytes("002")}); + TableName table = createTable(Bytes.toBytes("002")); //Add data and flush to create files in the two different regions putThreeRowsAndFlush(table); List regions = TEST_UTIL.getHBaseCluster().getRegions(table); @@ -232,10 +241,8 @@ private void validateDaughterRegionsFiles(HRegion region, String orignalFileName } private void verifyFilesAreTracked(Path regionDir, FileSystem fs) throws Exception { - String storeId = regionDir.getName() + "-info"; - for(FileStatus f : fs.listStatus(new Path(regionDir, Bytes.toString(FAMILY_NAME)))){ - assertTrue(TestStoreFileTracker.trackedFiles.get(storeId).stream().filter(s -> - s.getPath().equals(f.getPath())).findFirst().isPresent()); + for (FileStatus f : fs.listStatus(new Path(regionDir, FAMILY_NAME_STR))) { + assertTrue(TestStoreFileTracker.tracked(regionDir.getName(), FAMILY_NAME_STR, f.getPath())); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTracker.java index fc54eb057537..c89e151b40c6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTracker.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTracker.java @@ -20,11 +20,13 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.List; -import java.util.Map; - +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.LinkedBlockingQueue; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.regionserver.StoreContext; import org.apache.hadoop.hbase.regionserver.StoreFileInfo; import org.slf4j.Logger; @@ -33,7 +35,8 @@ public class TestStoreFileTracker extends DefaultStoreFileTracker { private static final Logger LOG = LoggerFactory.getLogger(TestStoreFileTracker.class); - public static Map> trackedFiles = new HashMap<>(); + private static ConcurrentMap> trackedFiles = + new ConcurrentHashMap<>(); private String storeId; public TestStoreFileTracker(Configuration conf, boolean isPrimaryReplica, StoreContext ctx) { @@ -41,7 +44,7 @@ public TestStoreFileTracker(Configuration conf, boolean isPrimaryReplica, StoreC if (ctx != null && ctx.getRegionFileSystem() != null) { this.storeId = ctx.getRegionInfo().getEncodedName() + "-" + ctx.getFamily().getNameAsString(); LOG.info("created storeId: {}", storeId); - trackedFiles.computeIfAbsent(storeId, v -> new ArrayList<>()); + trackedFiles.computeIfAbsent(storeId, v -> new LinkedBlockingQueue<>()); } else { LOG.info("ctx.getRegionFileSystem() returned null. Leaving storeId null."); } @@ -51,11 +54,19 @@ public TestStoreFileTracker(Configuration conf, boolean isPrimaryReplica, StoreC protected void doAddNewStoreFiles(Collection newFiles) throws IOException { LOG.info("adding to storeId: {}", storeId); trackedFiles.get(storeId).addAll(newFiles); - trackedFiles.putIfAbsent(storeId, (List)newFiles); } @Override public List load() throws IOException { - return trackedFiles.get(storeId); + return new ArrayList<>(trackedFiles.get(storeId)); + } + + public static boolean tracked(String encodedRegionName, String family, Path file) { + BlockingQueue files = trackedFiles.get(encodedRegionName + "-" + family); + return files != null && files.stream().anyMatch(s -> s.getPath().equals(file)); + } + + public static void clear() { + trackedFiles.clear(); } } From 29ee43be9dfbd865ea90523ea2555902e1b91ebd Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Thu, 6 Jan 2022 22:50:26 +0800 Subject: [PATCH 2/2] whitespace --- .../hbase/regionserver/TestMergesSplitsAddToTracker.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 index 588641a6209d..53956ed6b734 100644 --- 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 @@ -17,8 +17,7 @@ */ package org.apache.hadoop.hbase.regionserver; -import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory. - TRACKER_IMPL; +import static org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory.TRACKER_IMPL; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -67,7 +66,7 @@ public class TestMergesSplitsAddToTracker { HBaseClassTestRule.forClass(TestMergesSplitsAddToTracker.class); private static HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil(); - + private static final String FAMILY_NAME_STR = "info"; private static final byte[] FAMILY_NAME = Bytes.toBytes(FAMILY_NAME_STR);