You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by el...@apache.org on 2018/01/05 23:52:10 UTC

[1/2] hbase git commit: HBASE-19709 Ensure that we don't set a poolSize of 0

Repository: hbase
Updated Branches:
  refs/heads/branch-2 ce80e8e38 -> 443b27b70
  refs/heads/master a30d9fe8d -> 45e46bb24


HBASE-19709 Ensure that we don't set a poolSize of 0

In some situations, Runtime.getRuntime().getAvailableProcessors()
may return 0 which would result in calculatePoolSize returning 0
which will trigger an exception. Guard against this case.

Signed-off-by: Reid Chan <re...@outlook.com>
Signed-off-by: Chia-Ping Tsai <ch...@gmail.com>
Signed-off-by: Ted Yu <yu...@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/443b27b7
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/443b27b7
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/443b27b7

Branch: refs/heads/branch-2
Commit: 443b27b7074b866e3e077fc09c9c13aee1270577
Parents: ce80e8e
Author: Josh Elser <el...@apache.org>
Authored: Thu Jan 4 18:11:59 2018 -0500
Committer: Josh Elser <el...@apache.org>
Committed: Fri Jan 5 18:36:50 2018 -0500

----------------------------------------------------------------------
 .../hbase/master/cleaner/CleanerChore.java       | 10 ++++++++--
 .../hbase/master/cleaner/TestCleanerChore.java   | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/443b27b7/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
index 944db10..775d8f9 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
@@ -119,7 +119,7 @@ public abstract class CleanerChore<T extends FileCleanerDelegate> extends Schedu
    * @param poolSize size from configuration
    * @return size of pool after calculation
    */
-  private int calculatePoolSize(String poolSize) {
+  int calculatePoolSize(String poolSize) {
     if (poolSize.matches("[1-9][0-9]*")) {
       // If poolSize is an integer, return it directly,
       // but upmost to the number of available processors.
@@ -130,7 +130,13 @@ public abstract class CleanerChore<T extends FileCleanerDelegate> extends Schedu
       return size;
     } else if (poolSize.matches("0.[0-9]+|1.0")) {
       // if poolSize is a double, return poolSize * availableProcessors;
-      return (int) (AVAIL_PROCESSORS * Double.valueOf(poolSize));
+      // Ensure that we always return at least one.
+      int computedThreads = (int) (AVAIL_PROCESSORS * Double.valueOf(poolSize));
+      if (computedThreads < 1) {
+        LOG.debug("Computed {} threads for CleanerChore, using 1 instead", computedThreads);
+        return 1;
+      }
+      return computedThreads;
     } else {
       LOG.error("Unrecognized value: " + poolSize + " for " + CHORE_POOL_SIZE +
           ", use default config: " + DEFAULT_CHORE_POOL_SIZE + " instead.");

http://git-wip-us.apache.org/repos/asf/hbase/blob/443b27b7/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
index a9a856b..95178e2 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
@@ -388,6 +388,25 @@ public class TestCleanerChore {
     t.join();
   }
 
+  @Test
+  public void testMinimumNumberOfThreads() throws Exception {
+    Stoppable stop = new StoppableImplementation();
+    Configuration conf = UTIL.getConfiguration();
+    Path testDir = UTIL.getDataTestDir();
+    FileSystem fs = UTIL.getTestFileSystem();
+    String confKey = "hbase.test.cleaner.delegates";
+    conf.set(confKey, AlwaysDelete.class.getName());
+    conf.set(CleanerChore.CHORE_POOL_SIZE, "2");
+    AllValidPaths chore = new AllValidPaths("test-file-cleaner", stop, conf, fs, testDir, confKey);
+    int numProcs = Runtime.getRuntime().availableProcessors();
+    // Sanity
+    assertEquals(numProcs, chore.calculatePoolSize(Integer.toString(numProcs)));
+    // The implementation does not allow us to set more threads than we have processors
+    assertEquals(numProcs, chore.calculatePoolSize(Integer.toString(numProcs + 2)));
+    // Force us into the branch that is multiplying 0.0 against the number of processors
+    assertEquals(1, chore.calculatePoolSize("0.0"));
+  }
+
   private void createFiles(FileSystem fs, Path parentDir, int numOfFiles) throws IOException {
     Random random = new Random();
     for (int i = 0; i < numOfFiles; i++) {


[2/2] hbase git commit: HBASE-19709 Ensure that we don't set a poolSize of 0

Posted by el...@apache.org.
HBASE-19709 Ensure that we don't set a poolSize of 0

In some situations, Runtime.getRuntime().getAvailableProcessors()
may return 0 which would result in calculatePoolSize returning 0
which will trigger an exception. Guard against this case.

Signed-off-by: Reid Chan <re...@outlook.com>
Signed-off-by: Chia-Ping Tsai <ch...@gmail.com>
Signed-off-by: Ted Yu <yu...@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/45e46bb2
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/45e46bb2
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/45e46bb2

Branch: refs/heads/master
Commit: 45e46bb24253ea84f68047676b419276a5994fb7
Parents: a30d9fe
Author: Josh Elser <el...@apache.org>
Authored: Thu Jan 4 18:11:59 2018 -0500
Committer: Josh Elser <el...@apache.org>
Committed: Fri Jan 5 18:44:35 2018 -0500

----------------------------------------------------------------------
 .../hbase/master/cleaner/CleanerChore.java       | 10 ++++++++--
 .../hbase/master/cleaner/TestCleanerChore.java   | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/45e46bb2/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
index 944db10..775d8f9 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
@@ -119,7 +119,7 @@ public abstract class CleanerChore<T extends FileCleanerDelegate> extends Schedu
    * @param poolSize size from configuration
    * @return size of pool after calculation
    */
-  private int calculatePoolSize(String poolSize) {
+  int calculatePoolSize(String poolSize) {
     if (poolSize.matches("[1-9][0-9]*")) {
       // If poolSize is an integer, return it directly,
       // but upmost to the number of available processors.
@@ -130,7 +130,13 @@ public abstract class CleanerChore<T extends FileCleanerDelegate> extends Schedu
       return size;
     } else if (poolSize.matches("0.[0-9]+|1.0")) {
       // if poolSize is a double, return poolSize * availableProcessors;
-      return (int) (AVAIL_PROCESSORS * Double.valueOf(poolSize));
+      // Ensure that we always return at least one.
+      int computedThreads = (int) (AVAIL_PROCESSORS * Double.valueOf(poolSize));
+      if (computedThreads < 1) {
+        LOG.debug("Computed {} threads for CleanerChore, using 1 instead", computedThreads);
+        return 1;
+      }
+      return computedThreads;
     } else {
       LOG.error("Unrecognized value: " + poolSize + " for " + CHORE_POOL_SIZE +
           ", use default config: " + DEFAULT_CHORE_POOL_SIZE + " instead.");

http://git-wip-us.apache.org/repos/asf/hbase/blob/45e46bb2/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
index a9a856b..95178e2 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
@@ -388,6 +388,25 @@ public class TestCleanerChore {
     t.join();
   }
 
+  @Test
+  public void testMinimumNumberOfThreads() throws Exception {
+    Stoppable stop = new StoppableImplementation();
+    Configuration conf = UTIL.getConfiguration();
+    Path testDir = UTIL.getDataTestDir();
+    FileSystem fs = UTIL.getTestFileSystem();
+    String confKey = "hbase.test.cleaner.delegates";
+    conf.set(confKey, AlwaysDelete.class.getName());
+    conf.set(CleanerChore.CHORE_POOL_SIZE, "2");
+    AllValidPaths chore = new AllValidPaths("test-file-cleaner", stop, conf, fs, testDir, confKey);
+    int numProcs = Runtime.getRuntime().availableProcessors();
+    // Sanity
+    assertEquals(numProcs, chore.calculatePoolSize(Integer.toString(numProcs)));
+    // The implementation does not allow us to set more threads than we have processors
+    assertEquals(numProcs, chore.calculatePoolSize(Integer.toString(numProcs + 2)));
+    // Force us into the branch that is multiplying 0.0 against the number of processors
+    assertEquals(1, chore.calculatePoolSize("0.0"));
+  }
+
   private void createFiles(FileSystem fs, Path parentDir, int numOfFiles) throws IOException {
     Random random = new Random();
     for (int i = 0; i < numOfFiles; i++) {