This is an automated email from the ASF dual-hosted git repository.
liyu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 86b7b027b774fdac1f3c93707fcf32564b6ba588
Author: Yu Li <l...@apache.org>
AuthorDate: Wed Jun 15 00:47:28 2022 +0800
Revert "HBASE-27091 Speed up the loading of table descriptor from
filesystem (#4493)"
This reverts commit 5d0f4dc729a471c1dd0b926f0d6fabdbca01d7f7 to correct
signed-off info.
---
.../org/apache/hadoop/hbase/HBaseServerBase.java | 15 +---
.../org/apache/hadoop/hbase/TableDescriptors.java | 8 +--
.../org/apache/hadoop/hbase/master/HMaster.java | 1 -
.../hadoop/hbase/regionserver/HRegionServer.java | 1 -
.../hadoop/hbase/util/FSTableDescriptors.java | 80 +++-------------------
.../hadoop/hbase/util/TestFSTableDescriptors.java | 31 ---------
6 files changed, 12 insertions(+), 124 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java
index e148c7da84..f39e260914 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java
@@ -231,10 +231,8 @@ public abstract class HBaseServerBase<R extends
HBaseRpcServicesBase<?>> extends
// init the filesystem
this.dataFs = new HFileSystem(this.conf, useHBaseChecksum);
this.dataRootDir = CommonFSUtils.getRootDir(this.conf);
- int tableDescriptorParallelLoadThreads =
- conf.getInt("hbase.tabledescriptor.parallel.load.threads", 0);
this.tableDescriptors = new FSTableDescriptors(this.dataFs,
this.dataRootDir,
- !canUpdateTableDescriptor(), cacheTableDescriptor(),
tableDescriptorParallelLoadThreads);
+ !canUpdateTableDescriptor(), cacheTableDescriptor());
}
public HBaseServerBase(Configuration conf, String name) throws IOException {
@@ -468,17 +466,6 @@ public abstract class HBaseServerBase<R extends
HBaseRpcServicesBase<?>> extends
}
}
- protected final void closeTableDescriptors() {
- if (this.tableDescriptors != null) {
- LOG.info("Close table descriptors");
- try {
- this.tableDescriptors.close();
- } catch (IOException e) {
- LOG.debug("Failed to close table descriptors gracefully", e);
- }
- }
- }
-
/**
* In order to register ShutdownHook, this method is called when HMaster and
HRegionServer are
* started. For details, please refer to HBASE-26951
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java
index 9ecdf39679..1dc17eff0d 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java
@@ -17,7 +17,6 @@
*/
package org.apache.hadoop.hbase;
-import java.io.Closeable;
import java.io.IOException;
import java.util.Map;
import org.apache.hadoop.hbase.client.TableDescriptor;
@@ -27,7 +26,7 @@ import org.apache.yetus.audience.InterfaceAudience;
* Get, remove and modify table descriptors.
*/
@InterfaceAudience.Private
-public interface TableDescriptors extends Closeable {
+public interface TableDescriptors {
/**
* Test whether a given table exists, i.e, has a table descriptor.
@@ -36,11 +35,6 @@ public interface TableDescriptors extends Closeable {
return get(tableName) != null;
}
- @Override
- default void close() throws IOException {
- // do nothing by default
- }
-
/**
* @return TableDescriptor for tablename
*/
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
index 2b818d9cc2..c9556a80ed 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -601,7 +601,6 @@ public class HMaster extends
HBaseServerBase<MasterRpcServices> implements Maste
this.rpcServices.stop();
}
closeZooKeeper();
- closeTableDescriptors();
span.setStatus(StatusCode.OK);
} finally {
span.end();
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
index 1865929dc5..0ee4c5d746 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
@@ -977,7 +977,6 @@ public class HRegionServer extends
HBaseServerBase<RSRpcServices>
ZNodeClearer.deleteMyEphemeralNodeOnDisk();
closeZooKeeper();
- closeTableDescriptors();
LOG.info("Exiting; stopping=" + this.serverName + "; zookeeper
connection closed.");
span.setStatus(StatusCode.OK);
} finally {
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
index 27d66d14ca..8f9dd4426d 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
@@ -21,7 +21,6 @@ import com.google.errorprone.annotations.RestrictedApi;
import edu.umd.cs.findbugs.annotations.Nullable;
import java.io.EOFException;
import java.io.IOException;
-import java.io.InterruptedIOException;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
@@ -29,12 +28,6 @@ import java.util.Map;
import java.util.Optional;
import java.util.TreeMap;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentSkipListMap;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.commons.lang3.NotImplementedException;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FSDataInputStream;
@@ -62,7 +55,6 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.hbase.thirdparty.com.google.common.primitives.Ints;
-import
org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
/**
* Implementation of {@link TableDescriptors} that reads descriptors from the
passed filesystem. It
@@ -87,8 +79,6 @@ public class FSTableDescriptors implements TableDescriptors {
private final boolean fsreadonly;
private final boolean usecache;
private volatile boolean fsvisited;
- private boolean tableDescriptorParallelLoadEnable = false;
- private ThreadPoolExecutor executor;
long cachehits = 0;
long invocations = 0;
@@ -118,23 +108,10 @@ public class FSTableDescriptors implements
TableDescriptors {
public FSTableDescriptors(final FileSystem fs, final Path rootdir, final
boolean fsreadonly,
final boolean usecache) {
- this(fs, rootdir, fsreadonly, usecache, 0);
- }
-
- public FSTableDescriptors(final FileSystem fs, final Path rootdir, final
boolean fsreadonly,
- final boolean usecache, final int tableDescriptorParallelLoadThreads) {
this.fs = fs;
this.rootdir = rootdir;
this.fsreadonly = fsreadonly;
this.usecache = usecache;
- if (tableDescriptorParallelLoadThreads > 0) {
- tableDescriptorParallelLoadEnable = true;
- executor = new ThreadPoolExecutor(tableDescriptorParallelLoadThreads,
- tableDescriptorParallelLoadThreads, 1, TimeUnit.SECONDS, new
LinkedBlockingQueue<>(),
- new
ThreadFactoryBuilder().setNameFormat("FSTableDescriptorLoad-pool-%d").setDaemon(true)
-
.setUncaughtExceptionHandler(Threads.LOGGING_EXCEPTION_HANDLER).build());
- executor.allowCoreThreadTimeOut(true);
- }
}
public static void tryUpdateMetaTableDescriptor(Configuration conf) throws
IOException {
@@ -258,56 +235,27 @@ public class FSTableDescriptors implements
TableDescriptors {
*/
@Override
public Map<String, TableDescriptor> getAll() throws IOException {
- Map<String, TableDescriptor> tds = new ConcurrentSkipListMap<>();
+ Map<String, TableDescriptor> tds = new TreeMap<>();
if (fsvisited) {
for (Map.Entry<TableName, TableDescriptor> entry :
this.cache.entrySet()) {
tds.put(entry.getKey().getNameWithNamespaceInclAsString(),
entry.getValue());
}
} else {
- LOG.info("Fetching table descriptors from the filesystem.");
- final long startTime = EnvironmentEdgeManager.currentTime();
- AtomicBoolean allvisited = new AtomicBoolean(usecache);
- List<Path> tableDirs = FSUtils.getTableDirs(fs, rootdir);
- if (!tableDescriptorParallelLoadEnable) {
- for (Path dir : tableDirs) {
- internalGet(dir, tds, allvisited);
- }
- } else {
- CountDownLatch latch = new CountDownLatch(tableDirs.size());
- for (Path dir : tableDirs) {
- executor.submit(new Runnable() {
- @Override
- public void run() {
- try {
- internalGet(dir, tds, allvisited);
- } finally {
- latch.countDown();
- }
- }
- });
- }
- try {
- latch.await();
- } catch (InterruptedException ie) {
- throw (InterruptedIOException) new
InterruptedIOException().initCause(ie);
+ LOG.trace("Fetching table descriptors from the filesystem.");
+ boolean allvisited = usecache;
+ for (Path d : FSUtils.getTableDirs(fs, rootdir)) {
+ TableDescriptor htd = get(CommonFSUtils.getTableName(d));
+ if (htd == null) {
+ allvisited = false;
+ } else {
+ tds.put(htd.getTableName().getNameWithNamespaceInclAsString(), htd);
}
}
- fsvisited = allvisited.get();
- LOG.info("Fetched table descriptors(size=" + tds.size() + ") cost "
- + (EnvironmentEdgeManager.currentTime() - startTime) + "ms.");
+ fsvisited = allvisited;
}
return tds;
}
- private void internalGet(Path dir, Map<String, TableDescriptor> tds,
AtomicBoolean allvisited) {
- TableDescriptor htd = get(CommonFSUtils.getTableName(dir));
- if (htd == null) {
- allvisited.set(false);
- } else {
- tds.put(htd.getTableName().getNameWithNamespaceInclAsString(), htd);
- }
- }
-
/**
* Find descriptors by namespace.
* @see #get(org.apache.hadoop.hbase.TableName)
@@ -431,14 +379,6 @@ public class FSTableDescriptors implements
TableDescriptors {
return Bytes.toString(b);
}
- @Override
- public void close() throws IOException {
- // Close the executor when parallel loading enabled.
- if (tableDescriptorParallelLoadEnable) {
- this.executor.shutdown();
- }
- }
-
static final class SequenceIdAndFileLength {
final int sequenceId;
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSTableDescriptors.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSTableDescriptors.java
index 5e2b4b5295..24ca058dc2 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSTableDescriptors.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSTableDescriptors.java
@@ -285,32 +285,6 @@ public class TestFSTableDescriptors {
+ htds.getAll().size(), count + 1, htds.getAll().size());
}
- @Test
- public void testParallelGetAll() throws IOException, InterruptedException {
- final String name = "testParallelGetAll";
- FileSystem fs = FileSystem.get(UTIL.getConfiguration());
- // Enable parallel load table descriptor.
- FSTableDescriptors htds = new FSTableDescriptorsTest(fs, testDir, true,
20);
- final int count = 100;
- // Write out table infos.
- for (int i = 0; i < count; i++) {
- htds.createTableDescriptor(
- TableDescriptorBuilder.newBuilder(TableName.valueOf(name +
i)).build());
- }
- // add hbase:meta
- htds
-
.createTableDescriptor(TableDescriptorBuilder.newBuilder(TableName.META_TABLE_NAME).build());
-
- int getTableDescriptorSize = htds.getAll().size();
- assertEquals("getAll() didn't return all TableDescriptors, expected: " +
(count + 1) + " got: "
- + getTableDescriptorSize, count + 1, getTableDescriptorSize);
-
- // get again to check whether the cache works well
- getTableDescriptorSize = htds.getAll().size();
- assertEquals("getAll() didn't return all TableDescriptors with cache,
expected: " + (count + 1)
- + " got: " + getTableDescriptorSize, count + 1, getTableDescriptorSize);
- }
-
@Test
public void testGetAllOrdering() throws Exception {
FileSystem fs = FileSystem.get(UTIL.getConfiguration());
@@ -493,11 +467,6 @@ public class TestFSTableDescriptors {
super(fs, rootdir, false, usecache);
}
- public FSTableDescriptorsTest(FileSystem fs, Path rootdir, boolean
usecache,
- int tableDescriptorParallelLoadThreads) {
- super(fs, rootdir, false, usecache, tableDescriptorParallelLoadThreads);
- }
-
@Override
public TableDescriptor get(TableName tablename) {
LOG.info((super.isUsecache() ? "Cached" : "Non-Cached") + "
TableDescriptor.get() on "
