hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@apache.org
Subject svn commit: r1400286 - in /hbase/trunk/hbase-server/src: main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
Date Fri, 19 Oct 2012 21:02:02 GMT
Author: stack
Date: Fri Oct 19 21:02:01 2012
New Revision: 1400286

URL: http://svn.apache.org/viewvc?rev=1400286&view=rev
Log:
HBASE-6949 Automatically delete empty directories in CleanerChore

Added:
    hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
Modified:
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java?rev=1400286&r1=1400285&r2=1400286&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
(original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
Fri Oct 19 21:02:01 2012
@@ -121,7 +121,7 @@ public abstract class CleanerChore<T ext
       // loop over the found files and see if they should be deleted
       for (FileStatus file : files) {
         try {
-          if (file.isDir()) checkDirectory(file.getPath());
+          if (file.isDir()) checkAndDeleteDirectory(file.getPath());
           else checkAndDelete(file.getPath());
         } catch (IOException e) {
           e = RemoteExceptionHandler.checkIOException(e);
@@ -135,59 +135,40 @@ public abstract class CleanerChore<T ext
   }
 
   /**
-   * Check to see if we can delete a directory (and all the children files of that directory).
+   * Attempt to delete a directory and all files under that directory. Each child file is
passed
+   * through the delegates to see if it can be deleted. If the directory has no children
when the
+   * cleaners have finished it is deleted.
    * <p>
-   * A directory will not be deleted if it has children that are subsequently deleted since
that
-   * will require another set of lookups in the filesystem, which is semantically same as
waiting
-   * until the next time the chore is run, so we might as well wait.
-   * @param fs {@link FileSystem} where he directory resides
+   * If new children files are added between checks of the directory, the directory will
<b>not</b>
+   * be deleted.
    * @param toCheck directory to check
-   * @throws IOException
+   * @return <tt>true</tt> if the directory was deleted, <tt>false</tt>
otherwise.
+   * @throws IOException if there is an unexpected filesystem error
    */
-  private void checkDirectory(Path toCheck) throws IOException {
+  private boolean checkAndDeleteDirectory(Path toCheck) throws IOException {
     LOG.debug("Checking directory: " + toCheck);
-    FileStatus[] files = checkAndDeleteDirectory(toCheck);
+    FileStatus[] children = FSUtils.listStatus(fs, toCheck);
     // if the directory doesn't exist, then we are done
-    if (files == null) return;
+    if (children == null) return true;
 
-    // otherwise we need to check each of the child files
-    for (FileStatus file : files) {
-      Path filePath = file.getPath();
-      // if its a directory, then check to see if it should be deleted
-      if (file.isDir()) {
-        // check the subfiles to see if they can be deleted
-        checkDirectory(filePath);
-        continue;
+    boolean canDeleteThis = true;
+    for (FileStatus child : children) {
+      Path path = child.getPath();
+      // attempt to delete all the files under the directory
+      if (child.isDir()) {
+        if (!checkAndDeleteDirectory(path)) {
+          canDeleteThis = false;
+        }
       }
       // otherwise we can just check the file
-      checkAndDelete(filePath);
-    }
-
-    // recheck the directory to see if we can delete it this time
-    checkAndDeleteDirectory(toCheck);
-  }
-
-  /**
-   * Check and delete the passed directory if the directory is empty
-   * @param toCheck full path to the directory to check (and possibly delete)
-   * @return <tt>null</tt> if the directory was empty (and possibly deleted)
and otherwise an array
-   *         of <code>FileStatus</code> for the files in the directory
-   * @throws IOException
-   */
-  private FileStatus[] checkAndDeleteDirectory(Path toCheck) throws IOException {
-    LOG.debug("Attempting to delete directory:" + toCheck);
-    // if it doesn't exist, we are done
-    if (!fs.exists(toCheck)) return null;
-    // get the files below the directory
-    FileStatus[] files = FSUtils.listStatus(fs, toCheck, null);
-    // if there are no subfiles, then we can delete the directory
-    if (files == null) {
-      checkAndDelete(toCheck);
-      return null;
+      else if (!checkAndDelete(path)) {
+        canDeleteThis = false;
+      }
     }
 
-    // return the status of the files in the directory
-    return files;
+    // if all the children have been deleted, then we should try to delete this directory.
However,
+    // don't do so recursively so we don't delete files that have been added since we checked.
+    return canDeleteThis ? fs.delete(toCheck, false) : false;
   }
 
   /**
@@ -197,34 +178,40 @@ public abstract class CleanerChore<T ext
    * @throws IOException if cann't delete a file because of a filesystem issue
    * @throws IllegalArgumentException if the file is a directory and has children
    */
-  private void checkAndDelete(Path filePath) throws IOException, IllegalArgumentException
{
+  private boolean checkAndDelete(Path filePath) throws IOException, IllegalArgumentException
{
+    // first check to see if the path is valid
     if (!validate(filePath)) {
       LOG.warn("Found a wrongly formatted file: " + filePath.getName() + " deleting it.");
-      if (!this.fs.delete(filePath, true)) {
+      boolean success = this.fs.delete(filePath, true);
+      if(!success)
         LOG.warn("Attempted to delete:" + filePath
             + ", but couldn't. Run cleaner chain and attempt to delete on next pass.");
-      }
-      return;
+
+      return success;
     }
+
+    // check each of the cleaners for the file
     for (T cleaner : cleanersChain) {
-      if (cleaner.isStopped()) {
+      if (cleaner.isStopped() || this.stopper.isStopped()) {
         LOG.warn("A file cleaner" + this.getName() + " is stopped, won't delete any file
in:"
             + this.oldFileDir);
-        return;
+        return false;
       }
 
       if (!cleaner.isFileDeletable(filePath)) {
         // this file is not deletable, then we are done
         LOG.debug(filePath + " is not deletable according to:" + cleaner);
-        return;
+        return false;
       }
     }
     // delete this file if it passes all the cleaners
     LOG.debug("Removing:" + filePath + " from archive");
-    if (!this.fs.delete(filePath, false)) {
+    boolean success = this.fs.delete(filePath, false);
+    if (!success) {
       LOG.warn("Attempted to delete:" + filePath
           + ", but couldn't. Run cleaner chain and attempt to delete on next pass.");
     }
+    return success;
   }
 
 

Added: hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java?rev=1400286&view=auto
==============================================================================
--- hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
(added)
+++ hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
Fri Oct 19 21:02:01 2012
@@ -0,0 +1,255 @@
+/**
+ * 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.master.cleaner;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.SmallTests;
+import org.apache.hadoop.hbase.Stoppable;
+import org.apache.hadoop.hbase.util.FSUtils;
+import org.apache.hadoop.hbase.util.StoppableImplementation;
+import org.junit.After;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+@Category(SmallTests.class)
+public class TestCleanerChore {
+
+  private static final Log LOG = LogFactory.getLog(TestCleanerChore.class);
+  private static final HBaseTestingUtility UTIL = new HBaseTestingUtility();
+
+  @After
+  public void cleanup() throws Exception {
+    // delete and recreate the test directory, ensuring a clean test dir between tests
+    Path testDir = UTIL.getDataTestDir();
+    FileSystem fs = UTIL.getTestFileSystem();
+    fs.delete(testDir, true);
+    fs.mkdirs(testDir);
+  }
+
+  @Test
+  public void testSavesFilesOnRequest() 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, NeverDelete.class.getName());
+
+    AllValidPaths chore = new AllValidPaths("test-file-cleaner", stop, conf, fs, testDir,
confKey);
+
+    // create the directory layout in the directory to clean
+    Path parent = new Path(testDir, "parent");
+    Path file = new Path(parent, "someFile");
+    fs.mkdirs(parent);
+    // touch a new file
+    fs.create(file).close();
+    assertTrue("Test file didn't get created.", fs.exists(file));
+
+    // run the chore
+    chore.chore();
+
+    // verify all the files got deleted
+    assertTrue("File didn't get deleted", fs.exists(file));
+    assertTrue("Empty directory didn't get deleted", fs.exists(parent));
+  }
+
+  @Test
+  public void testDeletesEmptyDirectories() 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());
+
+    AllValidPaths chore = new AllValidPaths("test-file-cleaner", stop, conf, fs, testDir,
confKey);
+
+    // create the directory layout in the directory to clean
+    Path parent = new Path(testDir, "parent");
+    Path child = new Path(parent, "child");
+    Path file = new Path(child, "someFile");
+    fs.mkdirs(child);
+    // touch a new file
+    fs.create(file).close();
+    // also create a file in the top level directory
+    Path topFile = new Path(testDir, "topFile");
+    fs.create(topFile).close();
+    assertTrue("Test file didn't get created.", fs.exists(file));
+    assertTrue("Test file didn't get created.", fs.exists(topFile));
+
+    // run the chore
+    chore.chore();
+
+    // verify all the files got deleted
+    assertFalse("File didn't get deleted", fs.exists(topFile));
+    assertFalse("File didn't get deleted", fs.exists(file));
+    assertFalse("Empty directory didn't get deleted", fs.exists(child));
+    assertFalse("Empty directory didn't get deleted", fs.exists(parent));
+  }
+
+  /**
+   * Test to make sure that we don't attempt to ask the delegate whether or not we should
preserve a
+   * directory.
+   * @throws Exception on failure
+   */
+  @Test
+  public void testDoesNotCheckDirectories() 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());
+
+    AllValidPaths chore = new AllValidPaths("test-file-cleaner", stop, conf, fs, testDir,
confKey);
+    // spy on the delegate to ensure that we don't check for directories
+    AlwaysDelete delegate = (AlwaysDelete) chore.cleanersChain.get(0);
+    AlwaysDelete spy = Mockito.spy(delegate);
+    chore.cleanersChain.set(0, spy);
+
+    // create the directory layout in the directory to clean
+    Path parent = new Path(testDir, "parent");
+    Path file = new Path(parent, "someFile");
+    fs.mkdirs(parent);
+    // touch a new file
+    fs.create(file).close();
+    assertTrue("Test file didn't get created.", fs.exists(file));
+
+    chore.chore();
+    // make sure we never checked the directory
+    Mockito.verify(spy, Mockito.never()).isFileDeletable(parent);
+    Mockito.reset(spy);
+  }
+
+  @Test
+  public void testStoppedCleanerDoesNotDeleteFiles() 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());
+
+    AllValidPaths chore = new AllValidPaths("test-file-cleaner", stop, conf, fs, testDir,
confKey);
+
+    // also create a file in the top level directory
+    Path topFile = new Path(testDir, "topFile");
+    fs.create(topFile).close();
+    assertTrue("Test file didn't get created.", fs.exists(topFile));
+
+    // stop the chore
+    stop.stop("testing stop");
+
+    // run the chore
+    chore.chore();
+
+    // test that the file still exists
+    assertTrue("File got deleted while chore was stopped", fs.exists(topFile));
+  }
+
+  /**
+   * While cleaning a directory, all the files in the directory may be deleted, but there
may be
+   * another file added, in which case the directory shouldn't be deleted.
+   * @throws IOException on failure
+   */
+  @Test
+  public void testCleanerDoesNotDeleteDirectoryWithLateAddedFiles() throws IOException {
+    Stoppable stop = new StoppableImplementation();
+    Configuration conf = UTIL.getConfiguration();
+    final Path testDir = UTIL.getDataTestDir();
+    final FileSystem fs = UTIL.getTestFileSystem();
+    String confKey = "hbase.test.cleaner.delegates";
+    conf.set(confKey, AlwaysDelete.class.getName());
+
+    AllValidPaths chore = new AllValidPaths("test-file-cleaner", stop, conf, fs, testDir,
confKey);
+    // spy on the delegate to ensure that we don't check for directories
+    AlwaysDelete delegate = (AlwaysDelete) chore.cleanersChain.get(0);
+    AlwaysDelete spy = Mockito.spy(delegate);
+    chore.cleanersChain.set(0, spy);
+
+    // create the directory layout in the directory to clean
+    final Path parent = new Path(testDir, "parent");
+    Path file = new Path(parent, "someFile");
+    fs.mkdirs(parent);
+    // touch a new file
+    fs.create(file).close();
+    assertTrue("Test file didn't get created.", fs.exists(file));
+    final Path addedFile = new Path(parent, "addedFile");
+
+    // when we attempt to delete the original file, add another file in the same directory
+    Mockito.doAnswer(new Answer<Boolean>() {
+      @Override
+      public Boolean answer(InvocationOnMock invocation) throws Throwable {
+        fs.create(addedFile).close();
+        FSUtils.logFileSystemState(fs, testDir, LOG);
+        return (Boolean) invocation.callRealMethod();
+      }
+    }).when(spy).isFileDeletable(Mockito.any(Path.class));
+
+    // run the chore
+    chore.chore();
+
+    // make sure all the directories + added file exist, but the original file is deleted
+    assertTrue("Added file unexpectedly deleted", fs.exists(addedFile));
+    assertTrue("Parent directory deleted unexpectedly", fs.exists(parent));
+    assertFalse("Original file unexpectedly retained", fs.exists(file));
+    Mockito.verify(spy, Mockito.times(1)).isFileDeletable(Mockito.any(Path.class));
+    Mockito.reset(spy);
+  }
+
+  private static class AllValidPaths extends CleanerChore<BaseHFileCleanerDelegate>
{
+
+    public AllValidPaths(String name, Stoppable s, Configuration conf, FileSystem fs,
+        Path oldFileDir, String confkey) {
+      super(name, Integer.MAX_VALUE, s, conf, fs, oldFileDir, confkey);
+    }
+
+    // all paths are valid
+    @Override
+    protected boolean validate(Path file) {
+      return true;
+    }
+  };
+
+  public static class AlwaysDelete extends BaseHFileCleanerDelegate {
+    @Override
+    public boolean isFileDeletable(Path file) {
+      return true;
+    }
+  }
+
+  public static class NeverDelete extends BaseHFileCleanerDelegate {
+    @Override
+    public boolean isFileDeletable(Path file) {
+      return false;
+    }
+  }
+}
\ No newline at end of file



Mime
View raw message