From c1d101a0423f56eb60db8150741afe28d6de3922 Mon Sep 17 00:00:00 2001 From: Joseph Hwang Date: Mon, 11 Jul 2016 10:45:53 -0700 Subject: [PATCH] HBASE-16208 Check that the row exists before attempting to remove a queue from TableBasedReplicationQueuesImpl --- .../hbase/replication/TableBasedReplicationQueuesImpl.java | 13 +++++++++---- .../hbase/replication/TestReplicationStateHBaseImpl.java | 7 ++++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/TableBasedReplicationQueuesImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/TableBasedReplicationQueuesImpl.java index 28fa967..c665b66 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/TableBasedReplicationQueuesImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/TableBasedReplicationQueuesImpl.java @@ -93,12 +93,17 @@ public class TableBasedReplicationQueuesImpl extends ReplicationTableBase @Override public void removeQueue(String queueId) { - - try { + try (Table replicationTable = getOrBlockOnReplicationTable()){ byte[] rowKey = queueIdToRowKey(queueId); Delete deleteQueue = new Delete(rowKey); - safeQueueUpdate(deleteQueue); - } catch (IOException | ReplicationException e) { + boolean deleteSuccess = replicationTable.checkAndDelete(rowKey, + CF_QUEUE, COL_QUEUE_OWNER, CompareFilter.CompareOp.EQUAL, serverNameBytes, deleteQueue); + if (!deleteSuccess) { + LOG.info("No logs were registered for queue id=" + queueId + " with owner serverName=" + + serverName + " so no rows were removed from the replication table while removing the " + + "queue"); + } + } catch (IOException e) { String errMsg = "Failed removing queue queueId=" + queueId; abortable.abort(errMsg, e); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationStateHBaseImpl.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationStateHBaseImpl.java index ccae8a5..7ec6df8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationStateHBaseImpl.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationStateHBaseImpl.java @@ -84,7 +84,7 @@ public class TestReplicationStateHBaseImpl { conf.setClass("hbase.region.replica.replication.replicationQueues.class", TableBasedReplicationQueuesImpl.class, ReplicationQueues.class); conf.setClass("hbase.region.replica.replication.replicationQueuesClient.class", - TableBasedReplicationQueuesClientImpl.class, ReplicationQueuesClient.class); + TableBasedReplicationQueuesClientImpl.class, ReplicationQueuesClient.class); utility.startMiniCluster(); zkw = HBaseTestingUtility.getZooKeeperWatcher(utility); String replicationZNodeName = conf.get("zookeeper.znode.replication", "replication"); @@ -195,6 +195,11 @@ public class TestReplicationStateHBaseImpl { assertNull(rq1.getLogsInQueue("Queue1")); // Test that getting logs from a non-existent queue aborts assertEquals(6, ds1.getAbortCount()); + // Test removing a non-existent queue does not cause an abort. This is because we can + // attempt to remove a queue that has no corresponding Replication Table row (if we never + // registered a WAL for it) + rq1.removeQueue("NotHereQueue"); + assertEquals(6, ds1.getAbortCount()); } catch (ReplicationException e) { e.printStackTrace(); fail("testAddLog received a ReplicationException"); -- 2.8.0-rc2