From a6cd9d986de82615709170c4b0e68db3c640a977 Mon Sep 17 00:00:00 2001 From: Sameet Agarwal Date: Wed, 25 Feb 2015 14:45:29 -0800 Subject: [PATCH] HBASE-12924 HRegionServer#MovedRegionsCleaner Chore does not start Signed-off-by: Elliott Clark --- .../hbase/regionserver/HRegionServer.java | 20 +++- .../hadoop/hbase/TestMovedRegionsCleaner.java | 95 +++++++++++++++++++ 2 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/TestMovedRegionsCleaner.java 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 1b046dcceab..9ec15442ba4 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 @@ -787,7 +787,7 @@ public class HRegionServer extends HasThread implements this.leases = new Leases(this.threadWakeFrequency); // Create the thread to clean the moved regions list - movedRegionsCleaner = MovedRegionsCleaner.createAndStart(this); + movedRegionsCleaner = MovedRegionsCleaner.create(this); if (this.nonceManager != null) { // Create the scheduled chore that cleans up nonces. @@ -1636,6 +1636,7 @@ public class HRegionServer extends HasThread implements if (this.healthCheckChore != null) choreService.scheduleChore(healthCheckChore); if (this.nonceManagerChore != null) choreService.scheduleChore(nonceManagerChore); if (this.storefileRefresher != null) choreService.scheduleChore(storefileRefresher); + if (this.movedRegionsCleaner != null) choreService.scheduleChore(movedRegionsCleaner); // Leases is not a Thread. Internally it runs a daemon thread. If it gets // an unhandled exception, it will just exit. @@ -1981,6 +1982,7 @@ public class HRegionServer extends HasThread implements if (this.periodicFlusher != null) periodicFlusher.cancel(true); if (this.healthCheckChore != null) healthCheckChore.cancel(true); if (this.storefileRefresher != null) storefileRefresher.cancel(true); + if (this.movedRegionsCleaner != null) movedRegionsCleaner.cancel(true); if (this.cacheFlusher != null) { this.cacheFlusher.join(); @@ -2912,21 +2914,31 @@ public class HRegionServer extends HasThread implements } } + /* + * Use this to allow tests to override and schedule more frequently. + */ + + protected int movedRegionCleanerPeriod() { + return TIMEOUT_REGION_MOVED; + } + /** * Creates a Chore thread to clean the moved region cache. */ - protected static class MovedRegionsCleaner extends ScheduledChore implements Stoppable { + + protected final static class MovedRegionsCleaner extends ScheduledChore implements Stoppable { private HRegionServer regionServer; Stoppable stoppable; private MovedRegionsCleaner( HRegionServer regionServer, Stoppable stoppable){ - super("MovedRegionsCleaner for region " + regionServer, stoppable, TIMEOUT_REGION_MOVED); + super("MovedRegionsCleaner for region " + regionServer, stoppable, + regionServer.movedRegionCleanerPeriod()); this.regionServer = regionServer; this.stoppable = stoppable; } - static MovedRegionsCleaner createAndStart(HRegionServer rs){ + static MovedRegionsCleaner create(HRegionServer rs){ Stoppable stoppable = new Stoppable() { private volatile boolean isStopped = false; @Override public void stop(String why) { isStopped = true;} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMovedRegionsCleaner.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMovedRegionsCleaner.java new file mode 100644 index 00000000000..3480cbdaa45 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMovedRegionsCleaner.java @@ -0,0 +1,95 @@ +/** + * + * 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; + + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.io.IOException; + + +/** + * Test whether background cleanup of MovedRegion entries is happening + */ +@Category( MediumTests.class ) public class TestMovedRegionsCleaner { + + public static final Log LOG = LogFactory.getLog(TestRegionRebalancing.class); + private final HBaseTestingUtility UTIL = new HBaseTestingUtility(); + + public static int numCalls = 0; + + private static class TestMockRegionServer extends MiniHBaseCluster.MiniHBaseClusterRegionServer { + + public TestMockRegionServer(Configuration conf, CoordinatedStateManager cp) + throws IOException, InterruptedException { + super(conf, cp); + } + + protected int movedRegionCleanerPeriod() { + return 500; + } + + @Override protected void cleanMovedRegions() { + // count the number of calls that are being made to this + // + numCalls++; + super.cleanMovedRegions(); + } + } + + @After public void after() throws Exception { + UTIL.shutdownMiniCluster(); + } + + @Before public void before() throws Exception { + UTIL.getConfiguration() + .setStrings(HConstants.REGION_SERVER_IMPL, TestMockRegionServer.class.getName()); + UTIL.startMiniCluster(1); + } + + /** + * Start the cluster, wait for some time and verify that the background + * MovedRegion cleaner indeed gets called + * + * @throws IOException + * @throws InterruptedException + */ + @Test public void testMovedRegionsCleaner() throws IOException, InterruptedException { + // We need to sleep long enough to trigger at least one round of background calls + // to MovedRegionCleaner happen. Currently the period is set to 500ms. + // Setting the sleep here for 2s just to be safe + // + UTIL.waitFor(2000, new Waiter.Predicate() { + @Override + public boolean evaluate() throws IOException { + + // verify that there was at least one call to the cleanMovedRegions function + // + return numCalls > 0; + } + }); + } +}