From 9cfc0ee14ab3b3d72009fe47edb01f71a8094e15 Mon Sep 17 00:00:00 2001 From: Sruti Parthiban Date: Tue, 18 Aug 2020 11:52:56 -0700 Subject: [PATCH 1/3] Refactor ModifyCacheCapacityAction to follow builder pattern --- .../actions/ModifyCacheMaxSizeAction.java | 210 ++++++++++++------ .../deciders/CacheHealthDecider.java | 7 +- .../rca/store/rca/cache/CacheUtil.java | 3 + .../actions/ModifyCacheMaxSizeActionTest.java | 192 +++++++++------- .../deciders/CacheHealthDeciderTest.java | 7 - 5 files changed, 252 insertions(+), 167 deletions(-) diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java index f3b2e51b5..14c7210c8 100644 --- a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java +++ b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java @@ -16,14 +16,14 @@ package com.amazon.opendistro.elasticsearch.performanceanalyzer.decisionmaker.actions; import static com.amazon.opendistro.elasticsearch.performanceanalyzer.decisionmaker.actions.ImpactVector.Dimension.HEAP; +import static com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.rca.cache.CacheUtil.KB_TO_BYTES; +import static com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.rca.cache.CacheUtil.MB_TO_BYTES; import com.amazon.opendistro.elasticsearch.performanceanalyzer.AppContext; import com.amazon.opendistro.elasticsearch.performanceanalyzer.grpc.ResourceEnum; -import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.collector.NodeConfigCache; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.rca.cluster.NodeKey; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.rca.util.NodeConfigCacheReaderUtil; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.logging.log4j.LogManager; @@ -34,49 +34,44 @@ * deciders to implement actions like increasing the cache's size. Presently, it acts on field data * cache and shard request cache. */ - -// TODO: Split the cache action into separate actions for different caches. - public class ModifyCacheMaxSizeAction extends SuppressibleAction { private static final Logger LOG = LogManager.getLogger(ModifyCacheMaxSizeAction.class); - public static final String NAME = "modifyCacheCapacity"; - public static final long COOL_OFF_PERIOD_IN_MILLIS = 300 * 1_000; + public static final String NAME = "ModifyCacheMaxSize"; private final NodeKey esNode; private final ResourceEnum cacheType; - private final NodeConfigCache nodeConfigCache; - private final double cacheSizeUpperBound; + private final AppContext appContext; - private Long currentCacheMaxSizeInBytes; - private Long heapMaxSizeInBytes; - private long cacheUpperBoundInBytes; - private long desiredCacheMaxSizeInBytes; - - private Map stepSizeInBytes = new HashMap<>(); + private final long desiredCacheMaxSizeInBytes; + private final long currentCacheMaxSizeInBytes; + private final long coolOffPeriodInMillis; + private final boolean canUpdate; public ModifyCacheMaxSizeAction( final NodeKey esNode, final ResourceEnum cacheType, - final NodeConfigCache nodeConfigCache, - final double cacheSizeUpperBound, - final boolean increase, - final AppContext appContext) { - // TODO: Add lower bound for caches - // TODO: Address cache scaling down when JVM decider is available - + final AppContext appContext, + final long desiredCacheMaxSizeInBytes, + final long currentCacheMaxSizeInBytes, + final long coolOffPeriodInMillis, + final boolean canUpdate) { super(appContext); this.esNode = esNode; this.cacheType = cacheType; - this.nodeConfigCache = nodeConfigCache; - this.cacheSizeUpperBound = cacheSizeUpperBound; + this.appContext = appContext; - setStepSize(); + this.desiredCacheMaxSizeInBytes = desiredCacheMaxSizeInBytes; + this.currentCacheMaxSizeInBytes = currentCacheMaxSizeInBytes; + this.coolOffPeriodInMillis = coolOffPeriodInMillis; + this.canUpdate = canUpdate; + } - if (validateAndSetConfigValues()) { - long desiredCapacity = - increase ? currentCacheMaxSizeInBytes + getStepSize(cacheType) : currentCacheMaxSizeInBytes; - setDesiredCacheMaxSize(desiredCapacity); - } + public static Builder newBuilder( + final NodeKey esNode, + final ResourceEnum cacheType, + final AppContext appContext, + double upperBoundThreshold) { + return new Builder(esNode, cacheType, appContext, upperBoundThreshold); } @Override @@ -86,15 +81,12 @@ public String name() { @Override public boolean canUpdate() { - if (currentCacheMaxSizeInBytes == null) { - return false; - } - return desiredCacheMaxSizeInBytes != currentCacheMaxSizeInBytes; + return canUpdate && (desiredCacheMaxSizeInBytes != currentCacheMaxSizeInBytes); } @Override public long coolOffPeriodInMillis() { - return COOL_OFF_PERIOD_IN_MILLIS; + return coolOffPeriodInMillis; } @Override @@ -105,12 +97,10 @@ public List impactedNodes() { @Override public Map impact() { final ImpactVector impactVector = new ImpactVector(); - if (currentCacheMaxSizeInBytes != null) { - if (desiredCacheMaxSizeInBytes > currentCacheMaxSizeInBytes) { - impactVector.increasesPressure(HEAP); - } else if (desiredCacheMaxSizeInBytes < currentCacheMaxSizeInBytes) { - impactVector.decreasesPressure(HEAP); - } + if (desiredCacheMaxSizeInBytes > currentCacheMaxSizeInBytes) { + impactVector.increasesPressure(HEAP); + } else if (desiredCacheMaxSizeInBytes < currentCacheMaxSizeInBytes) { + impactVector.decreasesPressure(HEAP); } return Collections.singletonMap(esNode, impactVector); } @@ -133,15 +123,11 @@ public String toString() { return summary(); } - public void setStepSizeForCache(final long stepSizeInBytes) { - this.stepSizeInBytes.put(cacheType, stepSizeInBytes); - } - - public Long getCurrentCacheMaxSizeInBytes() { + public long getCurrentCacheMaxSizeInBytes() { return currentCacheMaxSizeInBytes; } - public Long getDesiredCacheMaxSizeInBytes() { + public long getDesiredCacheMaxSizeInBytes() { return desiredCacheMaxSizeInBytes; } @@ -149,37 +135,117 @@ public ResourceEnum getCacheType() { return cacheType; } - private boolean validateAndSetConfigValues() { - // This is intentionally not made static because different nodes can - // have different bounds based on instance types + public static final class Builder { + public static final long DEFAULT_COOL_OFF_PERIOD_IN_MILLIS = 300 * 1_000; + public static final boolean DEFAULT_IS_INCREASE = true; + public static final boolean DEFAULT_CAN_UPDATE = true; + + private final ResourceEnum cacheType; + private final NodeKey esNode; + private final AppContext appContext; + private double upperBoundThreshold; + + private long stepSizeInBytes; + private boolean isIncrease; + private boolean canUpdate; + private long coolOffPeriodInMillis; + + private Long currentCacheMaxSizeInBytes; + private Long desiredCacheMaxSizeInBytes; + private Long heapMaxSizeInBytes; + + private Builder( + final NodeKey esNode, + final ResourceEnum cacheType, + final AppContext appContext, + double upperBoundThreshold) { + this.esNode = esNode; + this.cacheType = cacheType; + this.appContext = appContext; + this.upperBoundThreshold = upperBoundThreshold; + + this.coolOffPeriodInMillis = DEFAULT_COOL_OFF_PERIOD_IN_MILLIS; + this.isIncrease = DEFAULT_IS_INCREASE; + this.canUpdate = DEFAULT_CAN_UPDATE; + + this.currentCacheMaxSizeInBytes = + NodeConfigCacheReaderUtil.readCacheMaxSizeInBytes( + appContext.getNodeConfigCache(), esNode, cacheType); + this.heapMaxSizeInBytes = + NodeConfigCacheReaderUtil.readHeapMaxSizeInBytes(appContext.getNodeConfigCache(), esNode); + this.desiredCacheMaxSizeInBytes = null; + setDefaultStepSize(cacheType); + } + + private void setDefaultStepSize(ResourceEnum cacheType) { + // TODO: Move configuration values to rca.conf + // TODO: Update the step size to also include percentage of heap size along with absolute + // value + switch (cacheType) { + case FIELD_DATA_CACHE: + // Field data cache having step size of 512MB + this.stepSizeInBytes = (long) 512 * MB_TO_BYTES; + break; + case SHARD_REQUEST_CACHE: + // Shard request cache step size of 512KB + this.stepSizeInBytes = (long) 512 * KB_TO_BYTES; + break; + default: + throw new IllegalArgumentException( + String.format("Unrecognizable cache type: [%s]", cacheType.toString())); + } + } - final Long cacheMaxSize = NodeConfigCacheReaderUtil.readCacheMaxSizeInBytes(nodeConfigCache, esNode, cacheType); - final Long heapMaxSize = NodeConfigCacheReaderUtil.readHeapMaxSizeInBytes(nodeConfigCache, esNode); + public Builder coolOffPeriod(long coolOffPeriodInMillis) { + this.coolOffPeriodInMillis = coolOffPeriodInMillis; + return this; + } - if (cacheMaxSize == null || cacheMaxSize == 0 || heapMaxSize == null || heapMaxSize == 0) { - return false; - } else { - currentCacheMaxSizeInBytes = cacheMaxSize; - heapMaxSizeInBytes = heapMaxSize; - cacheUpperBoundInBytes = (long) (heapMaxSizeInBytes * cacheSizeUpperBound); - return true; + public Builder increase(boolean isIncrease) { + this.isIncrease = isIncrease; + return this; } - } - private void setStepSize() { - // TODO: Update the step size to also include percentage of heap size along with absolute value - // Field data cache having step size of 512MB - stepSizeInBytes.put(ResourceEnum.FIELD_DATA_CACHE, 512 * 1_024L * 1_024L); + public Builder desiredCacheMaxSize(long desiredCacheMaxSizeInBytes) { + this.desiredCacheMaxSizeInBytes = desiredCacheMaxSizeInBytes; + return this; + } - // Shard request cache step size of 512KB - stepSizeInBytes.put(ResourceEnum.SHARD_REQUEST_CACHE, 512 * 1_024L); - } + public Builder stepSizeInBytes(long stepSizeInBytes) { + this.stepSizeInBytes = stepSizeInBytes; + return this; + } - private long getStepSize(final ResourceEnum cacheType) { - return stepSizeInBytes.get(cacheType); - } + public Builder upperBoundThreshold(double upperBoundThreshold) { + this.upperBoundThreshold = upperBoundThreshold; + return this; + } - private void setDesiredCacheMaxSize(final long desiredCacheMaxSize) { - this.desiredCacheMaxSizeInBytes = Math.min(desiredCacheMaxSize, cacheUpperBoundInBytes); + public ModifyCacheMaxSizeAction build() { + // In case of failure to read cache max size or heap max size from node config cache + // return an empty non-actionable action object + if (currentCacheMaxSizeInBytes == null || heapMaxSizeInBytes == null) { + LOG.error( + "Action: Fail to read cache max size or heap max size from node config cache. Return an non-actionable action"); + return new ModifyCacheMaxSizeAction( + esNode, cacheType, appContext, -1, -1, coolOffPeriodInMillis, false); + } + // skip the step size bound check if we set desiredCapacity + // explicitly in action builder + if (desiredCacheMaxSizeInBytes == null) { + desiredCacheMaxSizeInBytes = + isIncrease ? currentCacheMaxSizeInBytes + stepSizeInBytes : currentCacheMaxSizeInBytes; + } + long upperBoundInBytes = (long) (upperBoundThreshold * heapMaxSizeInBytes); + desiredCacheMaxSizeInBytes = Math.min(desiredCacheMaxSizeInBytes, upperBoundInBytes); + return new ModifyCacheMaxSizeAction( + esNode, + cacheType, + appContext, + desiredCacheMaxSizeInBytes, + currentCacheMaxSizeInBytes, + coolOffPeriodInMillis, + canUpdate); + } } } diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDecider.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDecider.java index 5299d0a39..f2bd68586 100644 --- a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDecider.java +++ b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDecider.java @@ -145,9 +145,10 @@ private ModifyCacheMaxSizeAction configureCacheMaxSize( final NodeKey esNode, final ResourceEnum cacheType, final boolean increase) { final double cacheUpperBound = getCacheUpperBound(cacheType); final ModifyCacheMaxSizeAction action = - new ModifyCacheMaxSizeAction( - esNode, cacheType, getAppContext().getNodeConfigCache(), cacheUpperBound, increase, - getAppContext()); + ModifyCacheMaxSizeAction + .newBuilder(esNode, cacheType, getAppContext(), getCacheUpperBound(cacheType)) + .increase(increase) + .build(); if (action.isActionable()) { return action; } diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/rca/cache/CacheUtil.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/rca/cache/CacheUtil.java index 6f29bd911..5d0a3cfd8 100644 --- a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/rca/cache/CacheUtil.java +++ b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/rca/cache/CacheUtil.java @@ -29,6 +29,9 @@ public class CacheUtil { private static final Logger LOG = LogManager.getLogger(CacheUtil.class); + public static final long KB_TO_BYTES = 1024; + public static final long MB_TO_BYTES = KB_TO_BYTES * 1024; + public static Double getTotalSizeInKB(final Metric cacheSizeGroupByOperation) { double totalSizeInKB = 0; diff --git a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeActionTest.java b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeActionTest.java index 0383a104f..b807ae4d8 100644 --- a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeActionTest.java +++ b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeActionTest.java @@ -41,57 +41,35 @@ public class ModifyCacheMaxSizeActionTest { private final long heapMaxSizeInBytes = 12000 * 1_000_000L; + private final long fieldDataCacheMaxSizeInBytes = 12000; + private final long shardRequestCacheMaxSizeInBytes = 12000; private AppContext appContext; @Before public void setUp() { - final long fieldDataCacheMaxSizeInBytes = 12000; - final long shardRequestCacheMaxSizeInBytes = 12000; - appContext = new AppContext(); - - appContext - .getNodeConfigCache() - .put( - new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")), - ResourceUtil.HEAP_MAX_SIZE, - heapMaxSizeInBytes); - appContext - .getNodeConfigCache() - .put( - new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")), - ResourceUtil.FIELD_DATA_CACHE_MAX_SIZE, - fieldDataCacheMaxSizeInBytes); - appContext - .getNodeConfigCache() - .put( - new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")), - ResourceUtil.SHARD_REQUEST_CACHE_MAX_SIZE, - shardRequestCacheMaxSizeInBytes); } @Test public void testIncreaseCapacity() { + populateNodeConfigCache(); NodeKey node1 = new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")); - ModifyCacheMaxSizeAction modifyCacheSizeAction = - new ModifyCacheMaxSizeAction( - node1, - ResourceEnum.FIELD_DATA_CACHE, - appContext.getNodeConfigCache(), - getDefaultFieldDataCacheUpperBound(), - true, - appContext); + ModifyCacheMaxSizeAction.Builder builder = + ModifyCacheMaxSizeAction.newBuilder( + node1, ResourceEnum.FIELD_DATA_CACHE, appContext, getDefaultFieldDataCacheUpperBound()); + ModifyCacheMaxSizeAction fieldDataCacheIncrease = builder.increase(true).build(); + assertTrue( - modifyCacheSizeAction.getDesiredCacheMaxSizeInBytes() - > modifyCacheSizeAction.getCurrentCacheMaxSizeInBytes()); - assertTrue(modifyCacheSizeAction.isActionable()); - assertEquals(300 * 1_000, modifyCacheSizeAction.coolOffPeriodInMillis()); - assertEquals(ResourceEnum.FIELD_DATA_CACHE, modifyCacheSizeAction.getCacheType()); - assertEquals(1, modifyCacheSizeAction.impactedNodes().size()); - - Map impact = modifyCacheSizeAction.impact().get(node1).getImpact(); + fieldDataCacheIncrease.getDesiredCacheMaxSizeInBytes() + > fieldDataCacheIncrease.getCurrentCacheMaxSizeInBytes()); + assertTrue(fieldDataCacheIncrease.isActionable()); + assertEquals(300 * 1_000, fieldDataCacheIncrease.coolOffPeriodInMillis()); + assertEquals(ResourceEnum.FIELD_DATA_CACHE, fieldDataCacheIncrease.getCacheType()); + assertEquals(1, fieldDataCacheIncrease.impactedNodes().size()); + + Map impact = fieldDataCacheIncrease.impact().get(node1).getImpact(); assertEquals(Impact.INCREASES_PRESSURE, impact.get(HEAP)); assertEquals(Impact.NO_IMPACT, impact.get(CPU)); assertEquals(Impact.NO_IMPACT, impact.get(NETWORK)); @@ -101,65 +79,53 @@ public void testIncreaseCapacity() { @Test public void testNoIncreaseCapacity() { + populateNodeConfigCache(); NodeKey node1 = new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")); - ModifyCacheMaxSizeAction modifyCacheSizeAction = - new ModifyCacheMaxSizeAction( - node1, - ResourceEnum.FIELD_DATA_CACHE, - appContext.getNodeConfigCache(), - getDefaultFieldDataCacheUpperBound(), - false, - appContext); + ModifyCacheMaxSizeAction.Builder builder = + ModifyCacheMaxSizeAction.newBuilder( + node1, ResourceEnum.FIELD_DATA_CACHE, appContext, getDefaultFieldDataCacheUpperBound()); + ModifyCacheMaxSizeAction fieldDataCacheNoIncrease = builder.increase(false).build(); assertEquals( - modifyCacheSizeAction.getDesiredCacheMaxSizeInBytes(), - modifyCacheSizeAction.getCurrentCacheMaxSizeInBytes()); - assertFalse(modifyCacheSizeAction.isActionable()); - assertEquals(300 * 1_000, modifyCacheSizeAction.coolOffPeriodInMillis()); - assertEquals(ResourceEnum.FIELD_DATA_CACHE, modifyCacheSizeAction.getCacheType()); - assertEquals(1, modifyCacheSizeAction.impactedNodes().size()); - - Map impact = modifyCacheSizeAction.impact().get(node1).getImpact(); - assertEquals(Impact.NO_IMPACT, impact.get(HEAP)); - assertEquals(Impact.NO_IMPACT, impact.get(CPU)); - assertEquals(Impact.NO_IMPACT, impact.get(NETWORK)); - assertEquals(Impact.NO_IMPACT, impact.get(RAM)); - assertEquals(Impact.NO_IMPACT, impact.get(DISK)); + fieldDataCacheNoIncrease.getDesiredCacheMaxSizeInBytes(), + fieldDataCacheNoIncrease.getCurrentCacheMaxSizeInBytes()); + assertFalse(fieldDataCacheNoIncrease.isActionable()); + assertEquals(300 * 1_000, fieldDataCacheNoIncrease.coolOffPeriodInMillis()); + assertEquals(ResourceEnum.FIELD_DATA_CACHE, fieldDataCacheNoIncrease.getCacheType()); + assertEquals(1, fieldDataCacheNoIncrease.impactedNodes().size()); + assertNoImpact(node1, fieldDataCacheNoIncrease); } @Test public void testBounds() { // TODO: Move to work with test rcaConf when bounds moved to nodeConfiguration rca + populateNodeConfigCache(); + long maxSizeInBytes = (long) (heapMaxSizeInBytes * getDefaultFieldDataCacheUpperBound()); setNodeConfigCache(ResourceUtil.FIELD_DATA_CACHE_MAX_SIZE, maxSizeInBytes); NodeKey node1 = new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")); - ModifyCacheMaxSizeAction fieldCacheIncrease = - new ModifyCacheMaxSizeAction( - node1, - ResourceEnum.FIELD_DATA_CACHE, - appContext.getNodeConfigCache(), - getDefaultFieldDataCacheUpperBound(), - true, - appContext); + ModifyCacheMaxSizeAction.Builder builder = + ModifyCacheMaxSizeAction.newBuilder( + node1, ResourceEnum.FIELD_DATA_CACHE, appContext, getDefaultFieldDataCacheUpperBound()); + ModifyCacheMaxSizeAction fieldDataCacheIncrease = builder.increase(false).build(); assertEquals( - fieldCacheIncrease.getDesiredCacheMaxSizeInBytes(), - fieldCacheIncrease.getCurrentCacheMaxSizeInBytes()); - assertFalse(fieldCacheIncrease.isActionable()); - assertNoImpact(node1, fieldCacheIncrease); + fieldDataCacheIncrease.getDesiredCacheMaxSizeInBytes(), + fieldDataCacheIncrease.getCurrentCacheMaxSizeInBytes()); + assertFalse(fieldDataCacheIncrease.isActionable()); + assertNoImpact(node1, fieldDataCacheIncrease); maxSizeInBytes = (long) (heapMaxSizeInBytes * getDefaultShardRequestCacheUpperBound()); setNodeConfigCache(ResourceUtil.SHARD_REQUEST_CACHE_MAX_SIZE, maxSizeInBytes); - ModifyCacheMaxSizeAction shardRequestCacheIncrease = - new ModifyCacheMaxSizeAction( + builder = + ModifyCacheMaxSizeAction.newBuilder( node1, ResourceEnum.SHARD_REQUEST_CACHE, - appContext.getNodeConfigCache(), - getDefaultShardRequestCacheUpperBound(), - true, - appContext); + appContext, + getDefaultShardRequestCacheUpperBound()); + ModifyCacheMaxSizeAction shardRequestCacheIncrease = builder.increase(true).build(); assertEquals( shardRequestCacheIncrease.getDesiredCacheMaxSizeInBytes(), shardRequestCacheIncrease.getCurrentCacheMaxSizeInBytes()); @@ -169,22 +135,57 @@ public void testBounds() { @Test public void testMutedAction() { + populateNodeConfigCache(); NodeKey node1 = new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")); - ModifyCacheMaxSizeAction modifyCacheSizeAction = - new ModifyCacheMaxSizeAction( - node1, - ResourceEnum.FIELD_DATA_CACHE, - appContext.getNodeConfigCache(), - getDefaultFieldDataCacheUpperBound(), - false, - appContext); + ModifyCacheMaxSizeAction.Builder builder = + ModifyCacheMaxSizeAction.newBuilder( + node1, ResourceEnum.FIELD_DATA_CACHE, appContext, getDefaultFieldDataCacheUpperBound()); + ModifyCacheMaxSizeAction modifyCacheSizeAction = builder.increase(true).build(); appContext.updateMutedActions(ImmutableSet.of(modifyCacheSizeAction.name())); assertFalse(modifyCacheSizeAction.isActionable()); } + @Test + public void testCacheMaxSizeNotPresent() { + setNodeConfigCache(ResourceUtil.HEAP_MAX_SIZE, heapMaxSizeInBytes); + NodeKey node1 = + new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")); + ModifyCacheMaxSizeAction.Builder builder = + ModifyCacheMaxSizeAction.newBuilder( + node1, ResourceEnum.FIELD_DATA_CACHE, appContext, getDefaultFieldDataCacheUpperBound()); + ModifyCacheMaxSizeAction fieldDataCacheNoAction = builder.increase(true).build(); + assertEquals( + fieldDataCacheNoAction.getDesiredCacheMaxSizeInBytes(), + fieldDataCacheNoAction.getCurrentCacheMaxSizeInBytes()); + assertFalse(fieldDataCacheNoAction.isActionable()); + assertEquals(300 * 1_000, fieldDataCacheNoAction.coolOffPeriodInMillis()); + assertEquals(ResourceEnum.FIELD_DATA_CACHE, fieldDataCacheNoAction.getCacheType()); + assertEquals(1, fieldDataCacheNoAction.impactedNodes().size()); + assertNoImpact(node1, fieldDataCacheNoAction); + } + + @Test + public void testHeapMaxSizeNotPresent() { + setNodeConfigCache(ResourceUtil.FIELD_DATA_CACHE_MAX_SIZE, fieldDataCacheMaxSizeInBytes); + NodeKey node1 = + new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")); + ModifyCacheMaxSizeAction.Builder builder = + ModifyCacheMaxSizeAction.newBuilder( + node1, ResourceEnum.FIELD_DATA_CACHE, appContext, getDefaultFieldDataCacheUpperBound()); + ModifyCacheMaxSizeAction fieldDataCacheNoAction = builder.increase(true).build(); + assertEquals( + fieldDataCacheNoAction.getDesiredCacheMaxSizeInBytes(), + fieldDataCacheNoAction.getCurrentCacheMaxSizeInBytes()); + assertFalse(fieldDataCacheNoAction.isActionable()); + assertEquals(300 * 1_000, fieldDataCacheNoAction.coolOffPeriodInMillis()); + assertEquals(ResourceEnum.FIELD_DATA_CACHE, fieldDataCacheNoAction.getCacheType()); + assertEquals(1, fieldDataCacheNoAction.impactedNodes().size()); + assertNoImpact(node1, fieldDataCacheNoAction); + } + private void assertNoImpact(NodeKey node, ModifyCacheMaxSizeAction modifyCacheSizeAction) { Map impact = modifyCacheSizeAction.impact().get(node).getImpact(); assertEquals(Impact.NO_IMPACT, impact.get(HEAP)); @@ -194,6 +195,27 @@ private void assertNoImpact(NodeKey node, ModifyCacheMaxSizeAction modifyCacheSi assertEquals(Impact.NO_IMPACT, impact.get(DISK)); } + private void populateNodeConfigCache() { + appContext + .getNodeConfigCache() + .put( + new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")), + ResourceUtil.HEAP_MAX_SIZE, + heapMaxSizeInBytes); + appContext + .getNodeConfigCache() + .put( + new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")), + ResourceUtil.FIELD_DATA_CACHE_MAX_SIZE, + fieldDataCacheMaxSizeInBytes); + appContext + .getNodeConfigCache() + .put( + new NodeKey(new InstanceDetails.Id("node-1"), new InstanceDetails.Ip("1.2.3.4")), + ResourceUtil.SHARD_REQUEST_CACHE_MAX_SIZE, + shardRequestCacheMaxSizeInBytes); + } + private void setNodeConfigCache(final Resource resource, final long maxSizeInBytes) { appContext .getNodeConfigCache() diff --git a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDeciderTest.java b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDeciderTest.java index 1be5aeabf..7c3e35567 100644 --- a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDeciderTest.java +++ b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDeciderTest.java @@ -15,11 +15,9 @@ package com.amazon.opendistro.elasticsearch.performanceanalyzer.decisionmaker.deciders; -import static com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.summaries.ResourceUtil.SEARCH_QUEUE_CAPACITY; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.MockitoAnnotations.initMocks; import com.amazon.opendistro.elasticsearch.performanceanalyzer.AppContext; import com.amazon.opendistro.elasticsearch.performanceanalyzer.decisionmaker.actions.Action; @@ -27,15 +25,11 @@ import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics.NodeRole; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.RcaTestHelper; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.Resources; -import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.flow_units.NodeConfigFlowUnit; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.summaries.HotNodeSummary; -import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.summaries.HotResourceSummary; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.summaries.ResourceUtil; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.core.RcaConf; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.util.InstanceDetails; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.util.RcaConsts; -import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.collector.NodeConfigClusterCollector; -import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.collector.NodeConfigCollector; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.rca.cluster.FieldDataCacheClusterRca; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.rca.cluster.NodeKey; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.rca.cluster.ShardRequestCacheClusterRca; @@ -49,7 +43,6 @@ import java.util.Map; import org.junit.Before; import org.junit.Test; -import org.mockito.Mockito; public class CacheHealthDeciderTest { private AppContext appContext; From d51ffb905059a7a8d4493440fa902a248a91907a Mon Sep 17 00:00:00 2001 From: Sruti Parthiban Date: Tue, 18 Aug 2020 15:05:55 -0700 Subject: [PATCH 2/3] Fix spotbug errors --- .../decisionmaker/actions/ModifyCacheMaxSizeAction.java | 6 ++++-- .../decisionmaker/deciders/CacheHealthDecider.java | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java index 14c7210c8..1a0a1ca7e 100644 --- a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java +++ b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java @@ -233,8 +233,10 @@ public ModifyCacheMaxSizeAction build() { // skip the step size bound check if we set desiredCapacity // explicitly in action builder if (desiredCacheMaxSizeInBytes == null) { - desiredCacheMaxSizeInBytes = - isIncrease ? currentCacheMaxSizeInBytes + stepSizeInBytes : currentCacheMaxSizeInBytes; + desiredCacheMaxSizeInBytes = currentCacheMaxSizeInBytes; + if (isIncrease) { + desiredCacheMaxSizeInBytes += currentCacheMaxSizeInBytes; + } } long upperBoundInBytes = (long) (upperBoundThreshold * heapMaxSizeInBytes); desiredCacheMaxSizeInBytes = Math.min(desiredCacheMaxSizeInBytes, upperBoundInBytes); diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDecider.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDecider.java index f2bd68586..8542c7a8d 100644 --- a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDecider.java +++ b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/CacheHealthDecider.java @@ -143,7 +143,6 @@ private Action getAction( private ModifyCacheMaxSizeAction configureCacheMaxSize( final NodeKey esNode, final ResourceEnum cacheType, final boolean increase) { - final double cacheUpperBound = getCacheUpperBound(cacheType); final ModifyCacheMaxSizeAction action = ModifyCacheMaxSizeAction .newBuilder(esNode, cacheType, getAppContext(), getCacheUpperBound(cacheType)) From 0d76956826e5b461e416d1e4588e3883ff37d9cc Mon Sep 17 00:00:00 2001 From: Sruti Parthiban Date: Tue, 18 Aug 2020 15:24:21 -0700 Subject: [PATCH 3/3] Fix spotbug error --- .../decisionmaker/actions/ModifyCacheMaxSizeAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java index 1a0a1ca7e..c042e0792 100644 --- a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java +++ b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/actions/ModifyCacheMaxSizeAction.java @@ -235,7 +235,7 @@ public ModifyCacheMaxSizeAction build() { if (desiredCacheMaxSizeInBytes == null) { desiredCacheMaxSizeInBytes = currentCacheMaxSizeInBytes; if (isIncrease) { - desiredCacheMaxSizeInBytes += currentCacheMaxSizeInBytes; + desiredCacheMaxSizeInBytes += stepSizeInBytes; } } long upperBoundInBytes = (long) (upperBoundThreshold * heapMaxSizeInBytes);