Repository: hbase Updated Branches: refs/heads/branch-1 4bae771b6 -> e5fb045aa
HBASE-15578 Handle HBASE-15234 for ReplicationHFileCleaner Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/e5fb045a Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e5fb045a Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e5fb045a Branch: refs/heads/branch-1 Commit: e5fb045aa9f56969f9ac0f444be90f92bde37af0 Parents: 4bae771 Author: Ashish Singhi <ashishsin...@apache.org> Authored: Mon Apr 4 15:02:19 2016 +0530 Committer: Ashish Singhi <ashishsin...@apache.org> Committed: Mon Apr 4 15:07:56 2016 +0530 ---------------------------------------------------------------------- .../master/ReplicationHFileCleaner.java | 48 +++++++++----- .../cleaner/TestReplicationHFileCleaner.java | 70 ++++++++++++++++++++ 2 files changed, 100 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/e5fb045a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationHFileCleaner.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationHFileCleaner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationHFileCleaner.java index 9bfea4b..5df9379 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationHFileCleaner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationHFileCleaner.java @@ -10,6 +10,7 @@ */ package org.apache.hadoop.hbase.replication.master; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -41,12 +42,11 @@ import org.apache.zookeeper.KeeperException; * deleting it from hfile archive directory. */ @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG) -public class ReplicationHFileCleaner extends BaseHFileCleanerDelegate implements Abortable { +public class ReplicationHFileCleaner extends BaseHFileCleanerDelegate { private static final Log LOG = LogFactory.getLog(ReplicationHFileCleaner.class); private ZooKeeperWatcher zkw; private ReplicationQueuesClient rqc; private boolean stopped = false; - private boolean aborted; @Override public Iterable<FileStatus> getDeletableFiles(Iterable<FileStatus> files) { @@ -129,18 +129,27 @@ public class ReplicationHFileCleaner extends BaseHFileCleanerDelegate implements // Make my own Configuration. Then I'll have my own connection to zk that // I can close myself when time comes. Configuration conf = new Configuration(config); + try { + setConf(conf, new ZooKeeperWatcher(conf, "replicationHFileCleaner", null)); + } catch (IOException e) { + LOG.error("Error while configuring " + this.getClass().getName(), e); + } + } + + @VisibleForTesting + public void setConf(Configuration conf, ZooKeeperWatcher zk) { super.setConf(conf); try { - initReplicationQueuesClient(conf); + initReplicationQueuesClient(conf, zk); } catch (IOException e) { LOG.error("Error while configuring " + this.getClass().getName(), e); } } - private void initReplicationQueuesClient(Configuration conf) + private void initReplicationQueuesClient(Configuration conf, ZooKeeperWatcher zk) throws ZooKeeperConnectionException, IOException { - this.zkw = new ZooKeeperWatcher(conf, "replicationHFileCleaner", null); - this.rqc = ReplicationFactory.getReplicationQueuesClient(zkw, conf, this); + this.zkw = zk; + this.rqc = ReplicationFactory.getReplicationQueuesClient(zkw, conf, new WarnOnlyAbortable()); } @Override @@ -161,18 +170,6 @@ public class ReplicationHFileCleaner extends BaseHFileCleanerDelegate implements } @Override - public void abort(String why, Throwable e) { - LOG.warn("Aborting ReplicationHFileCleaner because " + why, e); - this.aborted = true; - stop(why); - } - - @Override - public boolean isAborted() { - return this.aborted; - } - - @Override public boolean isFileDeletable(FileStatus fStat) { Set<String> hfileRefsFromQueue; // all members of this class are null if replication is disabled, @@ -190,4 +187,19 @@ public class ReplicationHFileCleaner extends BaseHFileCleanerDelegate implements } return !hfileRefsFromQueue.contains(fStat.getPath().getName()); } + + private static class WarnOnlyAbortable implements Abortable { + @Override + public void abort(String why, Throwable e) { + LOG.warn("ReplicationHFileCleaner received abort, ignoring. Reason: " + why); + if (LOG.isDebugEnabled()) { + LOG.debug(e); + } + } + + @Override + public boolean isAborted() { + return false; + } + } } http://git-wip-us.apache.org/repos/asf/hbase/blob/e5fb045a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestReplicationHFileCleaner.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestReplicationHFileCleaner.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestReplicationHFileCleaner.java index 87db386..a734885 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestReplicationHFileCleaner.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestReplicationHFileCleaner.java @@ -10,9 +10,14 @@ */ package org.apache.hadoop.hbase.master.cleaner; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.spy; + +import com.google.common.collect.Lists; import java.io.IOException; import java.lang.reflect.Field; @@ -26,12 +31,14 @@ 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.Abortable; import org.apache.hadoop.hbase.ChoreService; import org.apache.hadoop.hbase.CoordinatedStateManager; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.Server; import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.ZooKeeperConnectionException; import org.apache.hadoop.hbase.client.ClusterConnection; import org.apache.hadoop.hbase.replication.ReplicationException; import org.apache.hadoop.hbase.replication.ReplicationFactory; @@ -45,7 +52,10 @@ import org.apache.hadoop.hbase.replication.regionserver.Replication; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.zookeeper.MetaTableLocator; +import org.apache.hadoop.hbase.zookeeper.RecoverableZooKeeper; import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.data.Stat; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; @@ -201,6 +211,48 @@ public class TestReplicationHFileCleaner { cleaner.isFileDeletable(fs.getFileStatus(file)); } + /** + * ReplicationHFileCleaner should be able to ride over ZooKeeper errors without aborting. + */ + @Test + public void testZooKeeperAbort() throws Exception { + ReplicationHFileCleaner cleaner = new ReplicationHFileCleaner(); + + List<FileStatus> dummyFiles = + Lists.newArrayList(new FileStatus(100, false, 3, 100, System.currentTimeMillis(), new Path( + "hfile1")), new FileStatus(100, false, 3, 100, System.currentTimeMillis(), new Path( + "hfile2"))); + + FaultyZooKeeperWatcher faultyZK = + new FaultyZooKeeperWatcher(conf, "testZooKeeperAbort-faulty", null); + try { + faultyZK.init(); + cleaner.setConf(conf, faultyZK); + // should keep all files due to a ConnectionLossException getting the queues znodes + Iterable<FileStatus> toDelete = cleaner.getDeletableFiles(dummyFiles); + assertFalse(toDelete.iterator().hasNext()); + assertFalse(cleaner.isStopped()); + } finally { + faultyZK.close(); + } + + // when zk is working both files should be returned + cleaner = new ReplicationHFileCleaner(); + ZooKeeperWatcher zkw = new ZooKeeperWatcher(conf, "testZooKeeperAbort-normal", null); + try { + cleaner.setConf(conf, zkw); + Iterable<FileStatus> filesToDelete = cleaner.getDeletableFiles(dummyFiles); + Iterator<FileStatus> iter = filesToDelete.iterator(); + assertTrue(iter.hasNext()); + assertEquals(new Path("hfile1"), iter.next().getPath()); + assertTrue(iter.hasNext()); + assertEquals(new Path("hfile2"), iter.next().getPath()); + assertFalse(iter.hasNext()); + } finally { + zkw.close(); + } + } + static class DummyServer implements Server { @Override @@ -261,4 +313,22 @@ public class TestReplicationHFileCleaner { return null; } } + + static class FaultyZooKeeperWatcher extends ZooKeeperWatcher { + private RecoverableZooKeeper zk; + public FaultyZooKeeperWatcher(Configuration conf, String identifier, Abortable abortable) + throws ZooKeeperConnectionException, IOException { + super(conf, identifier, abortable); + } + + public void init() throws Exception { + this.zk = spy(super.getRecoverableZooKeeper()); + doThrow(new KeeperException.ConnectionLossException()) + .when(zk).getData("/hbase/replication/hfile-refs", null, new Stat()); + } + + public RecoverableZooKeeper getRecoverableZooKeeper() { + return zk; + } + } } \ No newline at end of file
