diff --git a/hbase-common/src/main/resources/hbase-default.xml b/hbase-common/src/main/resources/hbase-default.xml index e921fd499427..93a8d921470b 100644 --- a/hbase-common/src/main/resources/hbase-default.xml +++ b/hbase-common/src/main/resources/hbase-default.xml @@ -1964,6 +1964,7 @@ possible configurations would overwhelm and obscure the important. 300000 Timeout for regionservers to keep threads in snapshot request pool waiting. + Must be greater than 0. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/snapshot/RegionServerSnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/snapshot/RegionServerSnapshotManager.java index 8d26583a0df2..351fab614abe 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/snapshot/RegionServerSnapshotManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/snapshot/RegionServerSnapshotManager.java @@ -101,6 +101,17 @@ public class RegionServerSnapshotManager extends RegionServerProcedureManager { private ProcedureMemberRpcs memberRpcs; private ProcedureMember member; + static long getSnapshotTimeoutMillis(Configuration conf) { + long timeoutMillis = + conf.getLong(SNAPSHOT_TIMEOUT_MILLIS_KEY, SNAPSHOT_TIMEOUT_MILLIS_DEFAULT); + if (timeoutMillis > 0) { + return timeoutMillis; + } + LOG.warn("{} must be greater than 0, but is {}. Using default value {}", + SNAPSHOT_TIMEOUT_MILLIS_KEY, timeoutMillis, SNAPSHOT_TIMEOUT_MILLIS_DEFAULT); + return SNAPSHOT_TIMEOUT_MILLIS_DEFAULT; + } + /** * Exposed for testing. * @param conf HBase configuration. @@ -176,7 +187,7 @@ public Subprocedure buildSubprocedure(SnapshotDescription snapshot) { + snapshot.getTable() + " type " + snapshot.getType()); ForeignExceptionDispatcher exnDispatcher = new ForeignExceptionDispatcher(snapshot.getName()); Configuration conf = rss.getConfiguration(); - long timeoutMillis = conf.getLong(SNAPSHOT_TIMEOUT_MILLIS_KEY, SNAPSHOT_TIMEOUT_MILLIS_DEFAULT); + long timeoutMillis = getSnapshotTimeoutMillis(conf); long wakeMillis = conf.getLong(SNAPSHOT_REQUEST_WAKE_MILLIS_KEY, SNAPSHOT_REQUEST_WAKE_MILLIS_DEFAULT); @@ -267,8 +278,7 @@ static class SnapshotSubprocedurePool { SnapshotSubprocedurePool(String name, Configuration conf, Abortable abortable) { this.abortable = abortable; // configure the executor service - long keepAlive = conf.getLong(RegionServerSnapshotManager.SNAPSHOT_TIMEOUT_MILLIS_KEY, - RegionServerSnapshotManager.SNAPSHOT_TIMEOUT_MILLIS_DEFAULT); + long keepAlive = getSnapshotTimeoutMillis(conf); int threads = conf.getInt(CONCURENT_SNAPSHOT_TASKS_KEY, DEFAULT_CONCURRENT_SNAPSHOT_TASKS); this.name = name; executor = Threads.getBoundedCachedThreadPool(threads, keepAlive, TimeUnit.MILLISECONDS, @@ -383,7 +393,7 @@ public void initialize(RegionServerServices rss) throws KeeperException { // read in the snapshot request configuration properties Configuration conf = rss.getConfiguration(); - long keepAlive = conf.getLong(SNAPSHOT_TIMEOUT_MILLIS_KEY, SNAPSHOT_TIMEOUT_MILLIS_DEFAULT); + long keepAlive = getSnapshotTimeoutMillis(conf); int opThreads = conf.getInt(SNAPSHOT_REQUEST_THREADS_KEY, SNAPSHOT_REQUEST_THREADS_DEFAULT); // create the actual snapshot procedure member diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/snapshot/TestRegionServerSnapshotManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/snapshot/TestRegionServerSnapshotManager.java new file mode 100644 index 000000000000..244c919f93e8 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/snapshot/TestRegionServerSnapshotManager.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.regionserver.snapshot; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.testclassification.RegionServerTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +@Tag(RegionServerTests.TAG) +@Tag(SmallTests.TAG) +public class TestRegionServerSnapshotManager { + + @Test + public void testSnapshotTimeoutMustBePositive() { + Configuration conf = new Configuration(false); + + conf.setLong(RegionServerSnapshotManager.SNAPSHOT_TIMEOUT_MILLIS_KEY, -1); + assertEquals(RegionServerSnapshotManager.SNAPSHOT_TIMEOUT_MILLIS_DEFAULT, + RegionServerSnapshotManager.getSnapshotTimeoutMillis(conf)); + + conf.setLong(RegionServerSnapshotManager.SNAPSHOT_TIMEOUT_MILLIS_KEY, 0); + assertEquals(RegionServerSnapshotManager.SNAPSHOT_TIMEOUT_MILLIS_DEFAULT, + RegionServerSnapshotManager.getSnapshotTimeoutMillis(conf)); + + conf.setLong(RegionServerSnapshotManager.SNAPSHOT_TIMEOUT_MILLIS_KEY, 1); + assertEquals(1, RegionServerSnapshotManager.getSnapshotTimeoutMillis(conf)); + } +}