diff --git src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java index cd1fd5f..cccb707 100644 --- src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java +++ src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java @@ -29,6 +29,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ThreadPoolExecutor; +import com.google.common.annotations.VisibleForTesting; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.classification.InterfaceAudience; @@ -73,6 +74,7 @@ import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.FSTableDescriptors; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.zookeeper.KeeperException; +import org.joda.time.Days; /** * This class manages the procedure of taking and restoring snapshots. There is only one @@ -131,6 +133,19 @@ public class SnapshotManager implements Stoppable { /** number of current operations running on the master */ private static final int SNAPSHOT_POOL_THREADS_DEFAULT = 1; + /** + * Conf key to ensure that # of ms have elapsed before deleting the tmp directory if it is present + * during initialization + */ + private static final String SNAPSHOT_INPROGRESS_EXPIRATION_MILLIS_KEY = + "hbase.snapshot.inProgress.expiration.timeMillis"; + + /** + * By default, wait for (ms) before deleting the tmp directory on initialization (if exists) + */ + private static final long SNAPSHOT_INPROGRESS_EXPIRATION_MILLIS_DEFAULT = + Days.days(30).toStandardDuration().getMillis(); + private boolean stopped; private final MasterServices master; // Needed by TableEventHandlers private final MasterMetrics metricsMaster; @@ -264,9 +279,26 @@ public class SnapshotManager implements Stoppable { void resetTempDir() throws IOException { // cleanup any existing snapshots. Path tmpdir = SnapshotDescriptionUtils.getWorkingSnapshotDir(rootDir); - if (master.getMasterFileSystem().getFileSystem().exists(tmpdir)) { - if (!master.getMasterFileSystem().getFileSystem().delete(tmpdir, true)) { + FileSystem fileSystem = master.getMasterFileSystem().getFileSystem(); + long timeout = master.getConfiguration().getLong( + SNAPSHOT_INPROGRESS_EXPIRATION_MILLIS_KEY, SNAPSHOT_INPROGRESS_EXPIRATION_MILLIS_DEFAULT + ); + FileStatus[] snapshotsInProgress = FSUtils.listStatus(fileSystem, tmpdir); + if (snapshotsInProgress == null) { + return; + } + for (FileStatus snapshot : snapshotsInProgress) { + long lastModified = snapshot.getModificationTime(); + if (EnvironmentEdgeManager.currentTimeMillis() > (lastModified + timeout)) { + if (!fileSystem.delete(snapshot.getPath(), true)) { LOG.warn("Couldn't delete working snapshot directory: " + tmpdir); + } else { + LOG.warn("A working snapshot directory exists: " + snapshot.getPath() + " which was last modified: " + + lastModified + " deleting expired snapshot directory as threshold is: " + timeout + "(ms)"); + } + } else { + LOG.warn("A working snapshot directory exists: " + snapshot.getPath() + " which was last modified: " + + lastModified + " not deleting directory as threshold is: " + timeout + "(ms)"); } } } @@ -933,7 +965,7 @@ public class SnapshotManager implements Stoppable { * @throws UnsupportedOperationException in case cleaners are missing and * there're snapshot in the system */ - private void checkSnapshotSupport(final Configuration conf, final MasterFileSystem mfs) + @VisibleForTesting void checkSnapshotSupport(final Configuration conf, final MasterFileSystem mfs) throws IOException, UnsupportedOperationException { // Verify if snapshot is disabled by the user String enabled = conf.get(HBASE_SNAPSHOT_ENABLED); diff --git src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotManager.java src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotManager.java index 47c57bf..7fb4022 100644 --- src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotManager.java +++ src/test/java/org/apache/hadoop/hbase/master/snapshot/TestSnapshotManager.java @@ -20,10 +20,13 @@ package org.apache.hadoop.hbase.master.snapshot; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.*; import java.io.IOException; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseTestingUtility; @@ -37,7 +40,9 @@ import org.apache.hadoop.hbase.master.cleaner.HFileLinkCleaner; import org.apache.hadoop.hbase.master.metrics.MasterMetrics; import org.apache.hadoop.hbase.procedure.ProcedureCoordinator; import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils; +import org.apache.hadoop.hbase.util.FSUtils; import org.apache.zookeeper.KeeperException; +import org.joda.time.DateTime; import org.junit.Test; import org.junit.experimental.categories.Category; import org.mockito.Mockito; @@ -49,11 +54,11 @@ import org.mockito.Mockito; public class TestSnapshotManager { private static final HBaseTestingUtility UTIL = new HBaseTestingUtility(); - MasterServices services = Mockito.mock(MasterServices.class); - MasterMetrics metrics = Mockito.mock(MasterMetrics.class); - ProcedureCoordinator coordinator = Mockito.mock(ProcedureCoordinator.class); - ExecutorService pool = Mockito.mock(ExecutorService.class); - MasterFileSystem mfs = Mockito.mock(MasterFileSystem.class); + MasterServices services = mock(MasterServices.class); + MasterMetrics metrics = mock(MasterMetrics.class); + ProcedureCoordinator coordinator = mock(ProcedureCoordinator.class); + ExecutorService pool = mock(ExecutorService.class); + MasterFileSystem mfs = mock(MasterFileSystem.class); FileSystem fs; { try { @@ -70,10 +75,10 @@ public class TestSnapshotManager { private SnapshotManager getNewManager(final Configuration conf) throws IOException, KeeperException { Mockito.reset(services); - Mockito.when(services.getConfiguration()).thenReturn(conf); - Mockito.when(services.getMasterFileSystem()).thenReturn(mfs); - Mockito.when(mfs.getFileSystem()).thenReturn(fs); - Mockito.when(mfs.getRootDir()).thenReturn(UTIL.getDataTestDir()); + when(services.getConfiguration()).thenReturn(conf); + when(services.getMasterFileSystem()).thenReturn(mfs); + when(mfs.getFileSystem()).thenReturn(fs); + when(mfs.getRootDir()).thenReturn(UTIL.getDataTestDir()); return new SnapshotManager(services, metrics, coordinator, pool); } @@ -88,7 +93,7 @@ public class TestSnapshotManager { Mockito.when(handler.isFinished()).thenReturn(false); assertTrue("Manager isn't in process when handler is running", manager.isTakingSnapshot(tableName)); - Mockito.when(handler.isFinished()).thenReturn(true); + when(handler.isFinished()).thenReturn(true); assertFalse("Manager is process when handler isn't running", manager.isTakingSnapshot(tableName)); } @@ -151,6 +156,67 @@ public class TestSnapshotManager { } } + @Test + public void testDontDeleteTmpDirectoryUnlessExpired() throws Exception { + // tmp directory exists but is not expired (30 days default) + Path rootDir = UTIL.getDataTestDir(); + MasterServices masterServices = getServices(rootDir, new DateTime().minusDays(10).getMillis()); + new SnapshotManager(masterServices, metrics, coordinator, pool) { + @Override + void checkSnapshotSupport(Configuration conf, MasterFileSystem mfs) throws IOException, UnsupportedOperationException { + // do nothing + } + }; + FileSystem fileSystem = masterServices.getMasterFileSystem().getFileSystem(); + Path tmpDir = SnapshotDescriptionUtils.getWorkingSnapshotDir(rootDir); + verify(fileSystem, never()).delete(any(Path.class), eq(true)); + } + + @Test + public void testWeDeleteTmpSnapshotsIfExpired() throws Exception { + // directory exists but is expired > 30 days + Path rootDir = UTIL.getDataTestDir(); + MasterServices masterServices = getServices(rootDir, new DateTime().minusDays(40).getMillis()); + new SnapshotManager(masterServices, metrics, coordinator, pool) { + @Override + void checkSnapshotSupport(Configuration conf, MasterFileSystem mfs) throws IOException, UnsupportedOperationException { + // do nothing + } + }; + FileSystem fileSystem = masterServices.getMasterFileSystem().getFileSystem(); + Path tmpDir = SnapshotDescriptionUtils.getWorkingSnapshotDir(rootDir); + verify(fileSystem, times(1)).delete(eq(new Path(tmpDir, "EXPIRED")), eq(true)); // exists but too old, delete + verify(fileSystem, never()).delete(eq(new Path(tmpDir, "VALID")), eq(true)); // exists but too old, delete + } + + private MasterServices getServices(Path rootDir, long modifiedTimestamp) throws IOException { + Path tmpDir = SnapshotDescriptionUtils.getWorkingSnapshotDir(rootDir); + + + FileSystem fileSystem = mock(FileSystem.class); + when(fileSystem.exists(eq(tmpDir))).thenReturn(true); + + FileStatus expiredTmp = mock(FileStatus.class); + when(expiredTmp.getPath()).thenReturn(new Path(tmpDir, "EXPIRED")); + when(expiredTmp.getModificationTime()).thenReturn(modifiedTimestamp); + + FileStatus validTmp = mock(FileStatus.class); + when(validTmp.getPath()).thenReturn(new Path(tmpDir, "VALID")); + when(validTmp.getModificationTime()).thenReturn(System.currentTimeMillis()); + + when(fileSystem.listStatus(eq(tmpDir))).thenReturn(new FileStatus[]{expiredTmp, validTmp}); + + MasterFileSystem masterFileSystem = mock(MasterFileSystem.class); + when(masterFileSystem.getFileSystem()).thenReturn(fileSystem); + + when(masterFileSystem.getRootDir()).thenReturn(rootDir); + + MasterServices services = mock(MasterServices.class); + when(services.getMasterFileSystem()).thenReturn(masterFileSystem); + when(services.getConfiguration()).thenReturn(UTIL.getConfiguration()); + return services; + } + private boolean isSnapshotSupported(final SnapshotManager manager) { try { manager.checkSnapshotSupport();