From 35c6aee226e5639f0305b2b1cb0df98cb845bb78 Mon Sep 17 00:00:00 2001 From: Yingyi Zhang Date: Tue, 16 Apr 2019 14:39:46 -0700 Subject: [PATCH 1/4] Misc minor fixes in Helix tool, ReplicationEngine and Composite Manager 1. Add null check in getDatacenterName() in CompositeClusterManager. 2. Make instance config change ignore count more accurate. 3. Ensure Helix bootstrap/upgrade tool can exit in the end. 4. Break the loop if remoteReplicaInfo is found in getRemoteReplicaInfo(). --- .../CompositeClusterManager.java | 2 +- .../HelixBootstrapUpgradeUtil.java | 11 ++++++++++- .../HelixClusterManager.java | 5 +++-- .../ReplicationEngine.java | 1 + .../com.github.ambry.replication/ReplicationTest.java | 1 + .../clustermap/HelixBootstrapUpgradeTool.java | 5 ++++- 6 files changed, 20 insertions(+), 5 deletions(-) diff --git a/ambry-clustermap/src/main/java/com.github.ambry.clustermap/CompositeClusterManager.java b/ambry-clustermap/src/main/java/com.github.ambry.clustermap/CompositeClusterManager.java index f8e3e4d5d7..2be1f2456b 100644 --- a/ambry-clustermap/src/main/java/com.github.ambry.clustermap/CompositeClusterManager.java +++ b/ambry-clustermap/src/main/java/com.github.ambry.clustermap/CompositeClusterManager.java @@ -129,7 +129,7 @@ public byte getLocalDatacenterId() { @Override public String getDatacenterName(byte id) { String dcName = staticClusterManager.getDatacenterName(id); - if (!dcName.equals(helixClusterManager.getDatacenterName(id))) { + if (helixClusterManager != null && !dcName.equals(helixClusterManager.getDatacenterName(id))) { helixClusterManagerMetrics.getDatacenterNameMismatchCount.inc(); } return dcName; diff --git a/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixBootstrapUpgradeUtil.java b/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixBootstrapUpgradeUtil.java index 3671f5ed8c..8b7d8251c6 100644 --- a/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixBootstrapUpgradeUtil.java +++ b/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixBootstrapUpgradeUtil.java @@ -178,6 +178,8 @@ static void bootstrapOrUpgrade(String hardwareLayoutPath, String partitionLayout clusterMapToHelixMapper.validateAndClose(); } clusterMapToHelixMapper.logSummary(); + info("========Bootstrap or upgrade completed! (if program doesn't exit, please use Ctrl-c to terminate)========"); + System.exit(0); } /** @@ -421,7 +423,7 @@ private void populateInstancesAndPartitionsMap() { private void updateClusterMapInHelix(boolean startValidatingClusterManager) throws IOException { info("Initializing admins and possibly adding cluster in Helix (if non-existent)"); maybeAddCluster(); - info("Initialized, starting validating cluster manager"); + info("Validating cluster manager is {}", startValidatingClusterManager ? "ENABLED" : "DISABLED"); if (startValidatingClusterManager) { startClusterManager(); } @@ -1007,6 +1009,13 @@ private void logSummary() { } else { info("========No updates were done to the cluster in Helix========"); } + if (validatingHelixClusterManager != null) { + info("========Validating HelixClusterManager metrics========"); + info("Instance config change count: {}", + validatingHelixClusterManager.helixClusterManagerMetrics.instanceConfigChangeTriggerCount.getCount()); + info("Instance config update ignored count: {}", + validatingHelixClusterManager.helixClusterManagerMetrics.ignoredUpdatesCount.getCount()); + } } } diff --git a/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixClusterManager.java b/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixClusterManager.java index 06c412a64a..2d9e922046 100644 --- a/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixClusterManager.java +++ b/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixClusterManager.java @@ -38,11 +38,11 @@ import org.I0Itec.zkclient.IZkDataListener; import org.apache.helix.AccessOption; import org.apache.helix.HelixManager; -import org.apache.helix.InstanceConfigChangeListener; import org.apache.helix.InstanceType; -import org.apache.helix.LiveInstanceChangeListener; import org.apache.helix.NotificationContext; import org.apache.helix.ZNRecord; +import org.apache.helix.api.listeners.InstanceConfigChangeListener; +import org.apache.helix.api.listeners.LiveInstanceChangeListener; import org.apache.helix.model.InstanceConfig; import org.apache.helix.model.LiveInstance; import org.apache.helix.store.zk.ZkHelixPropertyStore; @@ -531,6 +531,7 @@ private void updateStateOfReplicas(List instanceConfigs) { logger.trace( "Ignoring instanceConfig change for partition {} on instance {} because partition override is enabled", partitionId, instanceName); + helixClusterManagerMetrics.ignoredUpdatesCount.inc(); } else { replica.setSealedState(sealedReplicas.contains(partitionId)); replica.setStoppedState(stoppedReplicas.contains(partitionId)); diff --git a/ambry-replication/src/main/java/com.github.ambry.replication/ReplicationEngine.java b/ambry-replication/src/main/java/com.github.ambry.replication/ReplicationEngine.java index 8ca96fa175..10ca74ad73 100644 --- a/ambry-replication/src/main/java/com.github.ambry.replication/ReplicationEngine.java +++ b/ambry-replication/src/main/java/com.github.ambry.replication/ReplicationEngine.java @@ -228,6 +228,7 @@ private RemoteReplicaInfo getRemoteReplicaInfo(PartitionId partitionId, String h .getHostname() .equals(hostName)) { foundRemoteReplicaInfo = remoteReplicaInfo; + break; } } // TODO: replace replicaPath.contains("vcr"). diff --git a/ambry-replication/src/test/java/com.github.ambry.replication/ReplicationTest.java b/ambry-replication/src/test/java/com.github.ambry.replication/ReplicationTest.java index 56608c107d..a863dd37df 100644 --- a/ambry-replication/src/test/java/com.github.ambry.replication/ReplicationTest.java +++ b/ambry-replication/src/test/java/com.github.ambry.replication/ReplicationTest.java @@ -1840,6 +1840,7 @@ List getRemoteReplicaInfos(Host remoteHost, StoreEventListene new RemoteReplicaInfo(peerReplicaId, replicaId, store, new MockFindToken(0, 0), Long.MAX_VALUE, SystemTime.getInstance(), new Port(peerReplicaId.getDataNodeId().getPort(), PortType.PLAINTEXT)); remoteReplicaInfos.add(remoteReplicaInfo); + break; } } } diff --git a/ambry-tools/src/main/java/com.github.ambry/clustermap/HelixBootstrapUpgradeTool.java b/ambry-tools/src/main/java/com.github.ambry/clustermap/HelixBootstrapUpgradeTool.java index b52a8366d3..fb55c6f04d 100644 --- a/ambry-tools/src/main/java/com.github.ambry/clustermap/HelixBootstrapUpgradeTool.java +++ b/ambry-tools/src/main/java/com.github.ambry/clustermap/HelixBootstrapUpgradeTool.java @@ -158,6 +158,9 @@ public static void main(String args[]) throws Exception { OptionSpecBuilder dryRun = parser.accepts("dryRun", "(Optional argument) Dry run, do not modify the cluster map in Helix."); + OptionSpecBuilder disableValidatingClusterManager = parser.accepts("disableVCM", + "(Optional argument) whether to disable validating cluster manager(VCM) in Helix bootstrap tool."); + OptionSet options = parser.parse(args); String hardwareLayoutPath = options.valueOf(hardwareLayoutPathOpt); String partitionLayoutPath = options.valueOf(partitionLayoutPathOpt); @@ -207,7 +210,7 @@ public static void main(String args[]) throws Exception { clusterNamePrefix, dcs, options.valueOf(maxPartitionsInOneResourceOpt) == null ? DEFAULT_MAX_PARTITIONS_PER_RESOURCE : Integer.valueOf(options.valueOf(maxPartitionsInOneResourceOpt)), options.has(dryRun), - options.has(forceRemove), new HelixAdminFactory(), true); + options.has(forceRemove), new HelixAdminFactory(), !options.has(disableValidatingClusterManager)); } } } From 37491478076c7ca7083cb83e6876b767cf2cd057 Mon Sep 17 00:00:00 2001 From: Yingyi Zhang Date: Tue, 16 Apr 2019 16:49:35 -0700 Subject: [PATCH 2/4] change log level of ignoring instanceConfig to TRACE --- .../java/com.github.ambry.clustermap/HelixClusterManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixClusterManager.java b/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixClusterManager.java index 2d9e922046..757c4de158 100644 --- a/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixClusterManager.java +++ b/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixClusterManager.java @@ -519,7 +519,7 @@ private void updateStateOfReplicas(List instanceConfigs) { AmbryDataNode node = instanceNameToAmbryDataNode.get(instanceName); if (instanceName.equals(selfInstanceName) || instanceXid <= currentXid.get()) { if (node == null) { - logger.info("Dynamic addition of new nodes is not yet supported, ignoring InstanceConfig {}", + logger.trace("Dynamic addition of new nodes is not yet supported, ignoring InstanceConfig {}", instanceConfig); } else { Set sealedReplicas = new HashSet<>(getSealedReplicas(instanceConfig)); From 20d30ef7ad4ed738c975563493baaec6f088eb65 Mon Sep 17 00:00:00 2001 From: Yingyi Zhang Date: Thu, 18 Apr 2019 09:59:55 -0700 Subject: [PATCH 3/4] minor change: move exit() to main method --- .../com.github.ambry.clustermap/HelixBootstrapUpgradeUtil.java | 2 -- .../com.github.ambry/clustermap/HelixBootstrapUpgradeTool.java | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixBootstrapUpgradeUtil.java b/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixBootstrapUpgradeUtil.java index 8b7d8251c6..f5dcd7fbab 100644 --- a/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixBootstrapUpgradeUtil.java +++ b/ambry-clustermap/src/main/java/com.github.ambry.clustermap/HelixBootstrapUpgradeUtil.java @@ -178,8 +178,6 @@ static void bootstrapOrUpgrade(String hardwareLayoutPath, String partitionLayout clusterMapToHelixMapper.validateAndClose(); } clusterMapToHelixMapper.logSummary(); - info("========Bootstrap or upgrade completed! (if program doesn't exit, please use Ctrl-c to terminate)========"); - System.exit(0); } /** diff --git a/ambry-tools/src/main/java/com.github.ambry/clustermap/HelixBootstrapUpgradeTool.java b/ambry-tools/src/main/java/com.github.ambry/clustermap/HelixBootstrapUpgradeTool.java index fb55c6f04d..b8b0aace93 100644 --- a/ambry-tools/src/main/java/com.github.ambry/clustermap/HelixBootstrapUpgradeTool.java +++ b/ambry-tools/src/main/java/com.github.ambry/clustermap/HelixBootstrapUpgradeTool.java @@ -212,6 +212,9 @@ public static void main(String args[]) throws Exception { : Integer.valueOf(options.valueOf(maxPartitionsInOneResourceOpt)), options.has(dryRun), options.has(forceRemove), new HelixAdminFactory(), !options.has(disableValidatingClusterManager)); } + System.out.println("======== HelixBootstrapUpgradeTool completed successfully! ========"); + System.out.println("( If program doesn't exit, please use Ctrl-c to terminate. )"); + System.exit(0); } } From 5bcd69386c88f9937520abcf41337c49fd68d909 Mon Sep 17 00:00:00 2001 From: Yingyi Zhang Date: Thu, 18 Apr 2019 13:59:45 -0700 Subject: [PATCH 4/4] remove deprecated listeners in MockHelixManager --- .../MockHelixManager.java | 111 +++++++++--------- .../ReplicationTest.java | 1 - 2 files changed, 54 insertions(+), 58 deletions(-) diff --git a/ambry-clustermap/src/test/java/com.github.ambry.clustermap/MockHelixManager.java b/ambry-clustermap/src/test/java/com.github.ambry.clustermap/MockHelixManager.java index 0eff5f09a7..846353b98f 100644 --- a/ambry-clustermap/src/test/java/com.github.ambry.clustermap/MockHelixManager.java +++ b/ambry-clustermap/src/test/java/com.github.ambry.clustermap/MockHelixManager.java @@ -19,26 +19,26 @@ import org.apache.helix.AccessOption; import org.apache.helix.ClusterMessagingService; import org.apache.helix.ConfigAccessor; -import org.apache.helix.ControllerChangeListener; -import org.apache.helix.CurrentStateChangeListener; -import org.apache.helix.ExternalViewChangeListener; import org.apache.helix.HelixAdmin; import org.apache.helix.HelixDataAccessor; import org.apache.helix.HelixManager; import org.apache.helix.HelixManagerProperties; -import org.apache.helix.IdealStateChangeListener; -import org.apache.helix.InstanceConfigChangeListener; import org.apache.helix.InstanceType; -import org.apache.helix.LiveInstanceChangeListener; import org.apache.helix.LiveInstanceInfoProvider; -import org.apache.helix.MessageListener; import org.apache.helix.NotificationContext; import org.apache.helix.PreConnectCallback; import org.apache.helix.PropertyKey; -import org.apache.helix.ScopedConfigChangeListener; import org.apache.helix.ZNRecord; import org.apache.helix.api.listeners.ClusterConfigChangeListener; +import org.apache.helix.api.listeners.ControllerChangeListener; +import org.apache.helix.api.listeners.CurrentStateChangeListener; +import org.apache.helix.api.listeners.ExternalViewChangeListener; +import org.apache.helix.api.listeners.IdealStateChangeListener; +import org.apache.helix.api.listeners.InstanceConfigChangeListener; +import org.apache.helix.api.listeners.LiveInstanceChangeListener; +import org.apache.helix.api.listeners.MessageListener; import org.apache.helix.api.listeners.ResourceConfigChangeListener; +import org.apache.helix.api.listeners.ScopedConfigChangeListener; import org.apache.helix.healthcheck.ParticipantHealthReportCollector; import org.apache.helix.model.HelixConfigScope; import org.apache.helix.model.LiveInstance; @@ -108,32 +108,21 @@ public void disconnect() { } @Override - public void addLiveInstanceChangeListener(LiveInstanceChangeListener listener) throws Exception { - if (beBadException != null) { - throw beBadException; - } - liveInstanceChangeListener = listener; - triggerLiveInstanceNotification(true); + public void addLiveInstanceChangeListener(org.apache.helix.LiveInstanceChangeListener listener) throws Exception { + // deprecated LiveInstanceChangeListener is no longer supported in testing + throw new IllegalStateException("Not implemented"); } @Override - public void addInstanceConfigChangeListener(InstanceConfigChangeListener listener) throws Exception { - if (beBadException != null) { - throw beBadException; - } - instanceConfigChangeListener = listener; - triggerConfigChangeNotification(true); + public void addInstanceConfigChangeListener(org.apache.helix.InstanceConfigChangeListener listener) throws Exception { + // deprecated InstanceConfigChangeListener is no longer supported in testing + throw new IllegalStateException("Not implemented"); } @Override - public void addExternalViewChangeListener(ExternalViewChangeListener listener) throws Exception { - if (beBadException != null) { - throw beBadException; - } - externalViewChangeListener = listener; - NotificationContext notificationContext = new NotificationContext(this); - notificationContext.setType(NotificationContext.Type.INIT); - externalViewChangeListener.onExternalViewChange(Collections.EMPTY_LIST, notificationContext); + public void addExternalViewChangeListener(org.apache.helix.ExternalViewChangeListener listener) throws Exception { + // deprecated ExternalViewChangeListener is no longer supported in testing + throw new IllegalStateException("Not implemented"); } @Override @@ -191,25 +180,27 @@ void triggerConfigChangeNotification(boolean init) { // Not implemented. //**************************** @Override - public void addIdealStateChangeListener(IdealStateChangeListener listener) throws Exception { + public void addIdealStateChangeListener(org.apache.helix.IdealStateChangeListener listener) throws Exception { throw new IllegalStateException("Not implemented"); } @Override - public void addIdealStateChangeListener( - org.apache.helix.api.listeners.IdealStateChangeListener idealStateChangeListener) throws Exception { + public void addIdealStateChangeListener(IdealStateChangeListener idealStateChangeListener) throws Exception { throw new IllegalStateException("Not implemented"); } @Override - public void addLiveInstanceChangeListener( - org.apache.helix.api.listeners.LiveInstanceChangeListener liveInstanceChangeListener) throws Exception { - throw new IllegalStateException("Not implemented"); + public void addLiveInstanceChangeListener(LiveInstanceChangeListener liveInstanceChangeListener) throws Exception { + if (beBadException != null) { + throw beBadException; + } + this.liveInstanceChangeListener = liveInstanceChangeListener; + triggerLiveInstanceNotification(true); } @Override - public void addConfigChangeListener(ScopedConfigChangeListener listener, HelixConfigScope.ConfigScopeProperty scope) - throws Exception { + public void addConfigChangeListener(org.apache.helix.ScopedConfigChangeListener listener, + HelixConfigScope.ConfigScopeProperty scope) throws Exception { throw new IllegalStateException("Not implemented"); } @@ -220,9 +211,13 @@ public void addConfigChangeListener(org.apache.helix.api.listeners.ConfigChangeL } @Override - public void addInstanceConfigChangeListener( - org.apache.helix.api.listeners.InstanceConfigChangeListener instanceConfigChangeListener) throws Exception { - throw new IllegalStateException("Not implemented"); + public void addInstanceConfigChangeListener(InstanceConfigChangeListener instanceConfigChangeListener) + throws Exception { + if (beBadException != null) { + throw beBadException; + } + this.instanceConfigChangeListener = instanceConfigChangeListener; + triggerConfigChangeNotification(true); } @Override @@ -237,65 +232,67 @@ public void addClusterfigChangeListener(ClusterConfigChangeListener clusterConfi } @Override - public void addConfigChangeListener( - org.apache.helix.api.listeners.ScopedConfigChangeListener scopedConfigChangeListener, + public void addConfigChangeListener(ScopedConfigChangeListener scopedConfigChangeListener, HelixConfigScope.ConfigScopeProperty configScopeProperty) throws Exception { throw new IllegalStateException("Not implemented"); } @Override - public void addMessageListener(org.apache.helix.api.listeners.MessageListener messageListener, String s) - throws Exception { + public void addMessageListener(MessageListener messageListener, String s) throws Exception { throw new IllegalStateException("Not implemented"); } @Override - public void addMessageListener(MessageListener listener, String instanceName) throws Exception { + public void addMessageListener(org.apache.helix.MessageListener listener, String instanceName) throws Exception { throw new IllegalStateException("Not implemented"); } @Override - public void addCurrentStateChangeListener( - org.apache.helix.api.listeners.CurrentStateChangeListener currentStateChangeListener, String s, String s1) + public void addCurrentStateChangeListener(CurrentStateChangeListener currentStateChangeListener, String s, String s1) throws Exception { throw new IllegalStateException("Not implemented"); } @Override - public void addCurrentStateChangeListener(CurrentStateChangeListener listener, String instanceName, String sessionId) - throws Exception { + public void addCurrentStateChangeListener(org.apache.helix.CurrentStateChangeListener listener, String instanceName, + String sessionId) throws Exception { throw new IllegalStateException("Not implemented"); } @Override - public void addExternalViewChangeListener( - org.apache.helix.api.listeners.ExternalViewChangeListener externalViewChangeListener) throws Exception { - throw new IllegalStateException("Not implemented"); + public void addExternalViewChangeListener(ExternalViewChangeListener externalViewChangeListener) throws Exception { + if (beBadException != null) { + throw beBadException; + } + this.externalViewChangeListener = externalViewChangeListener; + NotificationContext notificationContext = new NotificationContext(this); + notificationContext.setType(NotificationContext.Type.INIT); + this.externalViewChangeListener.onExternalViewChange(Collections.emptyList(), notificationContext); } @Override - public void addTargetExternalViewChangeListener( - org.apache.helix.api.listeners.ExternalViewChangeListener externalViewChangeListener) throws Exception { + public void addTargetExternalViewChangeListener(ExternalViewChangeListener externalViewChangeListener) + throws Exception { throw new IllegalStateException("Not implemented"); } @Override - public void addControllerListener(ControllerChangeListener listener) { + public void addControllerListener(org.apache.helix.ControllerChangeListener listener) { throw new IllegalStateException("Not implemented"); } @Override - public void addControllerListener(org.apache.helix.api.listeners.ControllerChangeListener controllerChangeListener) { + public void addControllerListener(ControllerChangeListener controllerChangeListener) { throw new IllegalStateException("Not implemented"); } @Override - public void addControllerMessageListener(org.apache.helix.api.listeners.MessageListener messageListener) { + public void addControllerMessageListener(MessageListener messageListener) { throw new IllegalStateException("Not implemented"); } @Override - public void addControllerMessageListener(MessageListener listener) { + public void addControllerMessageListener(org.apache.helix.MessageListener listener) { throw new IllegalStateException("Not implemented"); } diff --git a/ambry-replication/src/test/java/com.github.ambry.replication/ReplicationTest.java b/ambry-replication/src/test/java/com.github.ambry.replication/ReplicationTest.java index a863dd37df..56608c107d 100644 --- a/ambry-replication/src/test/java/com.github.ambry.replication/ReplicationTest.java +++ b/ambry-replication/src/test/java/com.github.ambry.replication/ReplicationTest.java @@ -1840,7 +1840,6 @@ List getRemoteReplicaInfos(Host remoteHost, StoreEventListene new RemoteReplicaInfo(peerReplicaId, replicaId, store, new MockFindToken(0, 0), Long.MAX_VALUE, SystemTime.getInstance(), new Port(peerReplicaId.getDataNodeId().getPort(), PortType.PLAINTEXT)); remoteReplicaInfos.add(remoteReplicaInfo); - break; } } }