You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2018/12/14 00:14:54 UTC
[26/50] [abbrv] hbase git commit: HBASE-20404 Fixes to CleanChore
correctness and operability.
HBASE-20404 Fixes to CleanChore correctness and operability.
* Make CleanerChore less chatty: move WARN message to DEBUG when we expect non-empty dirs
* Make CleanerChore less chatty: move IOE we'll retry to INFO
* CleanerChore should treat IOE for FileStatus as a failure
* Add tests asserting assumptions in above
Signed-off-by: Reid Chan <re...@outlook.com>
Signed-off-by: Mike Drob <md...@apache.org>
Conflicts:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java
hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java
hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/be4915eb
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/be4915eb
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/be4915eb
Branch: refs/heads/branch-1.3
Commit: be4915eb4f6604edd570b2bb937800e663f76043
Parents: 193d1dc
Author: Sean Busbey <bu...@apache.org>
Authored: Fri Apr 13 00:57:35 2018 -0500
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed Dec 12 18:08:19 2018 -0800
----------------------------------------------------------------------
.../hbase/master/cleaner/CleanerChore.java | 24 +++++++--
.../hbase/master/cleaner/TestCleanerChore.java | 54 ++++++++++++++++++--
.../apache/hadoop/hbase/util/TestFSUtils.java | 12 +++++
3 files changed, 84 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/be4915eb/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 dc614fb..5a4c407 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
@@ -33,6 +33,7 @@ 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.fs.PathIsNotEmptyDirectoryException;
import org.apache.hadoop.hbase.ScheduledChore;
import org.apache.hadoop.hbase.Stoppable;
import org.apache.hadoop.hbase.classification.InterfaceAudience;
@@ -406,6 +407,10 @@ public abstract class CleanerChore<T extends FileCleanerDelegate> extends Schedu
T act() throws IOException;
}
+ /**
+ * Attemps to clean up a directory, its subdirectories, and files.
+ * Return value is true if everything was deleted. false on partial / total failures.
+ */
private class CleanerTask extends RecursiveTask<Boolean> {
private final Path dir;
private final boolean root;
@@ -425,6 +430,8 @@ public abstract class CleanerChore<T extends FileCleanerDelegate> extends Schedu
List<FileStatus> subDirs;
final List<FileStatus> files;
try {
+ // if dir doesn't exist, we'll get null back for both of these
+ // which will fall through to succeeding.
subDirs = FSUtils.listStatusWithStatusFilter(fs, dir, new FileStatusFilter() {
@Override
public boolean accept(FileStatus f) {
@@ -438,8 +445,8 @@ public abstract class CleanerChore<T extends FileCleanerDelegate> extends Schedu
}
});
} catch (IOException ioe) {
- LOG.warn(dir + " doesn't exist, just skip it. ", ioe);
- return true;
+ LOG.warn("failed to get FileStatus for contents of '" + dir + "'", ioe);
+ return false;
}
boolean nullSubDirs = subDirs == null;
@@ -497,8 +504,19 @@ public abstract class CleanerChore<T extends FileCleanerDelegate> extends Schedu
try {
LOG.trace("Start deleting " + type + " under " + dir);
deleted = deletion.act();
+ } catch (PathIsNotEmptyDirectoryException exception) {
+ // N.B. HDFS throws this exception when we try to delete a non-empty directory, but
+ // LocalFileSystem throws a bare IOException. So some test code will get the verbose
+ // message below.
+ LOG.debug("Couldn't delete '" + dir + "' yet because it isn't empty. Probably transient. " +
+ "exception details at TRACE.");
+ LOG.trace("Couldn't delete '" + dir + "' yet because it isn't empty w/exception.",
+ exception);
+ deleted = false;
} catch (IOException ioe) {
- LOG.warn("Could not delete " + type + " under " + dir, ioe);
+ LOG.info("Could not delete " + type + " under " + dir + ". might be transient; we'll " +
+ "retry. if it keeps happening, use following exception when asking on mailing list.",
+ ioe);
deleted = false;
}
LOG.trace("Finish deleting " + type + " under " + dir + " deleted=" + deleted);
http://git-wip-us.apache.org/repos/asf/hbase/blob/be4915eb/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 505fd2c..7711354 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
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertTrue;
import java.io.IOException;
import java.util.Random;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -30,6 +31,7 @@ import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FSDataOutputStream;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FilterFileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.testclassification.SmallTests;
@@ -84,9 +86,55 @@ public class TestCleanerChore {
// 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));
+ // verify all the files were preserved
+ assertTrue("File shouldn't have been deleted", fs.exists(file));
+ assertTrue("directory shouldn't have been deleted", fs.exists(parent));
+ }
+
+ @Test
+ public void retriesIOExceptionInStatus() throws Exception {
+ Stoppable stop = new StoppableImplementation();
+ Configuration conf = UTIL.getConfiguration();
+ Path testDir = UTIL.getDataTestDir();
+ FileSystem fs = UTIL.getTestFileSystem();
+ String confKey = "hbase.test.cleaner.delegates";
+
+ Path child = new Path(testDir, "child");
+ Path file = new Path(child, "file");
+ fs.mkdirs(child);
+ fs.create(file).close();
+ assertTrue("test file didn't get created.", fs.exists(file));
+ final AtomicBoolean fails = new AtomicBoolean(true);
+
+ FilterFileSystem filtered = new FilterFileSystem(fs) {
+ public FileStatus[] listStatus(Path f) throws IOException {
+ if (fails.get()) {
+ throw new IOException("whomp whomp.");
+ }
+ return fs.listStatus(f);
+ }
+ };
+
+ AllValidPaths chore = new AllValidPaths("test-retry-ioe", stop, conf, filtered, testDir, confKey);
+
+ // trouble talking to the filesystem
+ Boolean result = chore.runCleaner();
+
+ // verify that it couldn't clean the files.
+ assertTrue("test rig failed to inject failure.", fs.exists(file));
+ assertTrue("test rig failed to inject failure.", fs.exists(child));
+ // and verify that it accurately reported the failure.
+ assertFalse("chore should report that it failed.", result);
+
+ // filesystem is back
+ fails.set(false);
+ result = chore.runCleaner();
+
+ // verify everything is gone.
+ assertFalse("file should have been destroyed.", fs.exists(file));
+ assertFalse("directory should have been destroyed.", fs.exists(child));
+ // and verify that it accurately reported success.
+ assertTrue("chore should claim it succeeded.", result);
}
@Test
http://git-wip-us.apache.org/repos/asf/hbase/blob/be4915eb/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java
index 2699292..96f4180 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java
@@ -300,6 +300,18 @@ public class TestFSUtils {
}
@Test
+ public void testFilteredStatusDoesNotThrowOnNotFound() throws Exception {
+ HBaseTestingUtility htu = new HBaseTestingUtility();
+ MiniDFSCluster cluster = htu.startMiniDFSCluster(1);
+ try {
+ assertNull(FSUtils.listStatusWithStatusFilter(cluster.getFileSystem(), new Path("definitely/doesn't/exist"), null));
+ } finally {
+ cluster.shutdown();
+ }
+
+ }
+
+ @Test
public void testRenameAndSetModifyTime() throws Exception {
HBaseTestingUtility htu = new HBaseTestingUtility();
Configuration conf = htu.getConfiguration();