From c641f6635b12e441c8c7f4377155f96a28e0d682 Mon Sep 17 00:00:00 2001 From: Guanghao Zhang Date: Fri, 12 Jan 2018 16:10:41 +0800 Subject: [PATCH] HBASE-19783 Change replication peer cluster key/endpoint from a not-null value to null is not allowed --- .../master/replication/ReplicationPeerManager.java | 31 +++++++++---- .../client/replication/TestReplicationAdmin.java | 54 ++++++++++++++++++++++ 2 files changed, 76 insertions(+), 9 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java index 696b2d7..a16ea8c 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java @@ -132,20 +132,19 @@ public class ReplicationPeerManager { checkPeerConfig(peerConfig); ReplicationPeerDescription desc = checkPeerExists(peerId); ReplicationPeerConfig oldPeerConfig = desc.getPeerConfig(); - if (!StringUtils.isBlank(peerConfig.getClusterKey()) && - !peerConfig.getClusterKey().equals(oldPeerConfig.getClusterKey())) { + if (!isStringEquals(peerConfig.getClusterKey(), oldPeerConfig.getClusterKey())) { throw new DoNotRetryIOException( "Changing the cluster key on an existing peer is not allowed. Existing key '" + - oldPeerConfig.getClusterKey() + "' for peer " + peerId + " does not match new key '" + - peerConfig.getClusterKey() + "'"); + oldPeerConfig.getClusterKey() + "' for peer " + peerId + " does not match new key '" + + peerConfig.getClusterKey() + "'"); } - if (!StringUtils.isBlank(peerConfig.getReplicationEndpointImpl()) && - !peerConfig.getReplicationEndpointImpl().equals(oldPeerConfig.getReplicationEndpointImpl())) { + if (!isStringEquals(peerConfig.getReplicationEndpointImpl(), + oldPeerConfig.getReplicationEndpointImpl())) { throw new DoNotRetryIOException("Changing the replication endpoint implementation class " + - "on an existing peer is not allowed. Existing class '" + - oldPeerConfig.getReplicationEndpointImpl() + "' for peer " + peerId + - " does not match new class '" + peerConfig.getReplicationEndpointImpl() + "'"); + "on an existing peer is not allowed. Existing class '" + + oldPeerConfig.getReplicationEndpointImpl() + "' for peer " + peerId + + " does not match new class '" + peerConfig.getReplicationEndpointImpl() + "'"); } } @@ -341,4 +340,18 @@ public class ReplicationPeerManager { return new ReplicationPeerManager(peerStorage, ReplicationStorageFactory.getReplicationQueueStorage(zk, conf), peers); } + + /** + * For replication peer cluster key or endpoint class, null and empty string is same. So here + * don't use {@link StringUtils#equals(CharSequence, CharSequence)} directly. + */ + private boolean isStringEquals(String s1, String s2) { + if (StringUtils.isBlank(s1)) { + return StringUtils.isBlank(s2); + } + if (StringUtils.isBlank(s2)) { + return false; + } + return s1.equals(s2); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/replication/TestReplicationAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/replication/TestReplicationAdmin.java index a6091e1..af4e362 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/replication/TestReplicationAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/replication/TestReplicationAdmin.java @@ -46,6 +46,8 @@ import org.apache.hadoop.hbase.replication.ReplicationPeerConfigBuilder; import org.apache.hadoop.hbase.replication.ReplicationPeerDescription; import org.apache.hadoop.hbase.replication.ReplicationQueueStorage; import org.apache.hadoop.hbase.replication.ReplicationStorageFactory; +import org.apache.hadoop.hbase.replication.TestReplicationEndpoint.InterClusterReplicationEndpointForTest; +import org.apache.hadoop.hbase.replication.regionserver.TestReplicator.ReplicationEndpointForTest; import org.apache.hadoop.hbase.testclassification.ClientTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.junit.After; @@ -846,4 +848,56 @@ public class TestReplicationAdmin { assertEquals(2097152, admin.getPeerConfig(ID_ONE).getBandwidth()); admin.removePeer(ID_ONE); } + + @Test + public void testPeerClusterKey() throws Exception { + ReplicationPeerConfigBuilder builder = ReplicationPeerConfig.newBuilder(); + builder.setClusterKey(KEY_ONE); + hbaseAdmin.addReplicationPeer(ID_ONE, builder.build()); + + try { + builder.setClusterKey(KEY_SECOND); + hbaseAdmin.updateReplicationPeerConfig(ID_ONE, builder.build()); + fail("Change cluster key on an existing peer is not allowed"); + } catch (Exception e) { + // OK + } + } + + @Test + public void testPeerReplicationEndpointImpl() throws Exception { + ReplicationPeerConfigBuilder builder = ReplicationPeerConfig.newBuilder(); + builder.setClusterKey(KEY_ONE); + builder.setReplicationEndpointImpl(ReplicationEndpointForTest.class.getName()); + hbaseAdmin.addReplicationPeer(ID_ONE, builder.build()); + + try { + builder.setReplicationEndpointImpl(InterClusterReplicationEndpointForTest.class.getName()); + hbaseAdmin.updateReplicationPeerConfig(ID_ONE, builder.build()); + fail("Change replication endpoint implementation class on an existing peer is not allowed"); + } catch (Exception e) { + // OK + } + + try { + builder = ReplicationPeerConfig.newBuilder(); + builder.setClusterKey(KEY_ONE); + hbaseAdmin.updateReplicationPeerConfig(ID_ONE, builder.build()); + fail("Change replication endpoint implementation class on an existing peer is not allowed"); + } catch (Exception e) { + // OK + } + + builder = ReplicationPeerConfig.newBuilder(); + builder.setClusterKey(KEY_SECOND); + hbaseAdmin.addReplicationPeer(ID_SECOND, builder.build()); + + try { + builder.setReplicationEndpointImpl(ReplicationEndpointForTest.class.getName()); + hbaseAdmin.updateReplicationPeerConfig(ID_SECOND, builder.build()); + fail("Change replication endpoint implementation class on an existing peer is not allowed"); + } catch (Exception e) { + // OK + } + } } -- 1.9.1