Repository: ambari Updated Branches: refs/heads/branch-2.5 4d6a71b79 -> a57b24a09
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/a57b24a0 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/a57b24a0 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/a57b24a0 Branch: refs/heads/branch-2.5 Commit: a57b24a09d9a3e01dba4a2badc2082aa5f4409c3 Parents: 4d6a71b Author: Jonathan Hurley <[email protected]> Authored: Mon Apr 24 13:04:00 2017 -0400 Committer: Jonathan Hurley <[email protected]> Committed: Mon Apr 24 13:12:57 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/a57b24a0/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 3d304ea..5b592c7 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 @@ -118,13 +118,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 @@ -133,14 +138,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/a57b24a0/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 9594803..56c2b36 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(); @@ -754,4 +754,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/a57b24a0/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 6b39afa..666d3ea 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 @@ -335,6 +335,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) @@ -1027,22 +1034,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) { @@ -1086,6 +1102,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/a57b24a0/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 9d339e2..a61b7b1 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();
