Repository: ambari Updated Branches: refs/heads/trunk 8c039bbc2 -> 2958fbefb
AMBARI-20833 - Calculation of Effective Cluster Version During a Large Upgrade is Inefficient (jonathanhurley) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/2958fbef Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/2958fbef Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/2958fbef Branch: refs/heads/trunk Commit: 2958fbefb069be22a2e6140a102ec87e6b7e1ad5 Parents: 8c039bb Author: Jonathan Hurley <[email protected]> Authored: Mon Apr 24 13:04:00 2017 -0400 Committer: Jonathan Hurley <[email protected]> Committed: Mon Apr 24 13:04:00 2017 -0400 ---------------------------------------------------------------------- .../upgrades/UpdateDesiredStackAction.java | 14 +++-- .../org/apache/ambari/server/state/Cluster.java | 11 +++- .../server/state/cluster/ClusterImpl.java | 54 ++++++++++++++------ .../cluster/ClusterEffectiveVersionTest.java | 2 + 4 files changed, 60 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/2958fbef/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpdateDesiredStackAction.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpdateDesiredStackAction.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpdateDesiredStackAction.java index 134288f..7bcb9d0 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpdateDesiredStackAction.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/UpdateDesiredStackAction.java @@ -119,13 +119,18 @@ public class UpdateDesiredStackAction extends AbstractServerAction { LOG.warn(String.format("Did not receive role parameter %s, will save configs using anonymous username %s", ServerAction.ACTION_USER_NAME, userName)); } - return updateDesiredStack(clusterName, originalStackId, targetStackId, version, direction, upgradePack, userName); + // invalidate any cached effective ID + Cluster cluster = clusters.getCluster(clusterName); + cluster.invalidateUpgradeEffectiveVersion(); + + return updateDesiredStack(cluster, originalStackId, targetStackId, version, direction, + upgradePack, userName); } /** * Set the cluster's Desired Stack Id during an upgrade. * - * @param clusterName the name of the cluster the action is meant for + * @param cluster the cluster * @param originalStackId the stack Id of the cluster before the upgrade. * @param targetStackId the stack Id that was desired for this upgrade. * @param direction direction, either upgrade or downgrade @@ -134,14 +139,15 @@ public class UpdateDesiredStackAction extends AbstractServerAction { * @return the command report to return */ private CommandReport updateDesiredStack( - String clusterName, StackId originalStackId, StackId targetStackId, + Cluster cluster, StackId originalStackId, StackId targetStackId, String version, Direction direction, UpgradePack upgradePack, String userName) throws AmbariException, InterruptedException { + + String clusterName = cluster.getClusterName(); StringBuilder out = new StringBuilder(); StringBuilder err = new StringBuilder(); try { - Cluster cluster = clusters.getCluster(clusterName); StackId currentClusterStackId = cluster.getCurrentStackVersion(); out.append(String.format("Params: %s %s %s %s %s %s\n", clusterName, originalStackId.getStackId(), targetStackId.getStackId(), version, direction.getText(false), upgradePack.getName())); http://git-wip-us.apache.org/repos/asf/ambari/blob/2958fbef/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java b/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java index 49fc8c0..cf28ea4 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java @@ -675,10 +675,10 @@ public interface Cluster { * Gets an {@link UpgradeEntity} if there is an upgrade in progress or an * upgrade that has been suspended. This will return the associated * {@link UpgradeEntity} if it exists. - * + * * @return an upgrade which will either be in progress or suspended, or * {@code null} if none. - * + * */ UpgradeEntity getUpgradeInProgress(); @@ -753,4 +753,11 @@ public interface Cluster { */ void addSuspendedUpgradeParameters(Map<String, String> commandParams, Map<String, String> roleParams); + + /** + * Invalidates any cached effective cluster versions for upgrades. + * + * @see #getEffectiveClusterVersion() + */ + void invalidateUpgradeEffectiveVersion(); } http://git-wip-us.apache.org/repos/asf/ambari/blob/2958fbef/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java index b86c5cd..da1a1ef 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java @@ -334,6 +334,13 @@ public class ClusterImpl implements Cluster { */ private Map<String, String> m_clusterPropertyCache = new ConcurrentHashMap<>(); + /** + * A simple cache of the effective cluster version during an upgrade. Since + * calculation of this during an upgrade is not very quick or clean, it's good + * to cache it. + */ + private final Map<Long, String> upgradeEffectiveVersionCache = new ConcurrentHashMap<>(); + @Inject public ClusterImpl(@Assisted ClusterEntity clusterEntity, Injector injector, AmbariEventPublisher eventPublisher) @@ -1026,22 +1033,31 @@ public class ClusterImpl implements Cluster { return getCurrentClusterVersion(); } - String effectiveVersion = null; - switch (upgradeEntity.getUpgradeType()) { - case NON_ROLLING: - if (upgradeEntity.getDirection() == Direction.UPGRADE) { - boolean pastChangingStack = isNonRollingUpgradePastUpgradingStack(upgradeEntity); - effectiveVersion = pastChangingStack ? upgradeEntity.getToVersion() : upgradeEntity.getFromVersion(); - } else { - // Should be the lower value during a Downgrade. + // see if this is in the cache first, and only walk the upgrade if it's not + Long upgradeId = upgradeEntity.getId(); + String effectiveVersion = upgradeEffectiveVersionCache.get(upgradeId); + if (null == effectiveVersion) { + switch (upgradeEntity.getUpgradeType()) { + case NON_ROLLING: + if (upgradeEntity.getDirection() == Direction.UPGRADE) { + boolean pastChangingStack = isNonRollingUpgradePastUpgradingStack(upgradeEntity); + effectiveVersion = pastChangingStack ? upgradeEntity.getToVersion() + : upgradeEntity.getFromVersion(); + } else { + // Should be the lower value during a Downgrade. + effectiveVersion = upgradeEntity.getToVersion(); + } + break; + case ROLLING: + default: + // Version will be higher on upgrade and lower on downgrade + // directions. effectiveVersion = upgradeEntity.getToVersion(); - } - break; - case ROLLING: - default: - // Version will be higher on upgrade and lower on downgrade directions. - effectiveVersion = upgradeEntity.getToVersion(); - break; + break; + } + + // cache for later use + upgradeEffectiveVersionCache.put(upgradeId, effectiveVersion); } if (effectiveVersion == null) { @@ -1085,6 +1101,14 @@ public class ClusterImpl implements Cluster { } /** + * {@inheritDoc} + */ + @Override + public void invalidateUpgradeEffectiveVersion() { + upgradeEffectiveVersionCache.clear(); + } + + /** * Get all of the ClusterVersionEntity objects for the cluster. * @return */ http://git-wip-us.apache.org/repos/asf/ambari/blob/2958fbef/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterEffectiveVersionTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterEffectiveVersionTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterEffectiveVersionTest.java index d3c8acf..bba197f 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterEffectiveVersionTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterEffectiveVersionTest.java @@ -143,6 +143,7 @@ public class ClusterEffectiveVersionTest extends EasyMockSupport { Cluster clusterSpy = Mockito.spy(m_cluster); UpgradeEntity upgradeEntity = createNiceMock(UpgradeEntity.class); + EasyMock.expect(upgradeEntity.getId()).andReturn(1L).atLeastOnce(); EasyMock.expect(upgradeEntity.getUpgradeType()).andReturn(UpgradeType.ROLLING).atLeastOnce(); EasyMock.expect(upgradeEntity.getFromVersion()).andReturn("2.3.0.0-1234").anyTimes(); EasyMock.expect(upgradeEntity.getToVersion()).andReturn("2.4.0.0-1234").atLeastOnce(); @@ -184,6 +185,7 @@ public class ClusterEffectiveVersionTest extends EasyMockSupport { // from/to are switched on downgrade UpgradeEntity upgradeEntity = createNiceMock(UpgradeEntity.class); + EasyMock.expect(upgradeEntity.getId()).andReturn(1L).atLeastOnce(); EasyMock.expect(upgradeEntity.getUpgradeType()).andReturn(UpgradeType.NON_ROLLING).atLeastOnce(); EasyMock.expect(upgradeEntity.getToVersion()).andReturn("2.3.0.0-1234").atLeastOnce(); EasyMock.expect(upgradeEntity.getFromVersion()).andReturn("2.4.0.0-1234").anyTimes();
