diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a351a1061e15..92134a92998c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add an individual setting of rate limiter for segment replication ([#12959](https://github.com/opensearch-project/OpenSearch/pull/12959)) - [Streaming Indexing] Ensure support of the new transport by security plugin ([#13174](https://github.com/opensearch-project/OpenSearch/pull/13174)) - Add cluster setting to dynamically configure the buckets for filter rewrite optimization. ([#13179](https://github.com/opensearch-project/OpenSearch/pull/13179)) +- [Tiered Caching] Gate new stats logic behind FeatureFlags.PLUGGABLE_CACHE ([#13238](https://github.com/opensearch-project/OpenSearch/pull/13238)) - [Tiered Caching] Add a dynamic setting to disable/enable disk cache. ([#13373](https://github.com/opensearch-project/OpenSearch/pull/13373)) - [Remote Store] Add capability of doing refresh as determined by the translog ([#12992](https://github.com/opensearch-project/OpenSearch/pull/12992)) - [Tiered caching] Make Indices Request Cache Stale Key Mgmt Threshold setting dynamic ([#12941](https://github.com/opensearch-project/OpenSearch/pull/12941)) diff --git a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java index 185d51732a116..eea13ce70ccb5 100644 --- a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java +++ b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java @@ -25,6 +25,7 @@ import org.opensearch.common.cache.serializer.ICacheKeySerializer; import org.opensearch.common.cache.serializer.Serializer; import org.opensearch.common.cache.stats.CacheStatsHolder; +import org.opensearch.common.cache.stats.DefaultCacheStatsHolder; import org.opensearch.common.cache.stats.ImmutableCacheStatsHolder; import org.opensearch.common.cache.store.builders.ICacheBuilder; import org.opensearch.common.cache.store.config.CacheConfig; @@ -162,7 +163,8 @@ private EhcacheDiskCache(Builder builder) { this.ehCacheEventListener = new EhCacheEventListener(builder.getRemovalListener(), builder.getWeigher()); this.cache = buildCache(Duration.ofMillis(expireAfterAccess.getMillis()), builder); List dimensionNames = Objects.requireNonNull(builder.dimensionNames, "Dimension names can't be null"); - this.cacheStatsHolder = new CacheStatsHolder(dimensionNames); + // If this cache is being used, FeatureFlags.PLUGGABLE_CACHE is already on, so we can always use the DefaultCacheStatsHolder. + this.cacheStatsHolder = new DefaultCacheStatsHolder(dimensionNames); } @SuppressWarnings({ "rawtypes" }) diff --git a/server/src/main/java/org/opensearch/common/cache/stats/CacheStatsHolder.java b/server/src/main/java/org/opensearch/common/cache/stats/CacheStatsHolder.java index a8b7c27ef9e79..a1cfb8d806af3 100644 --- a/server/src/main/java/org/opensearch/common/cache/stats/CacheStatsHolder.java +++ b/server/src/main/java/org/opensearch/common/cache/stats/CacheStatsHolder.java @@ -8,288 +8,31 @@ package org.opensearch.common.cache.stats; -import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import java.util.TreeMap; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; -import java.util.function.Consumer; /** - * A class ICache implementations use to internally keep track of their stats across multiple dimensions. - * Not intended to be exposed outside the cache; for this, caches use getImmutableCacheStatsHolder() to create an immutable - * copy of the current state of the stats. - * Currently, in the IRC, the stats tracked in a CacheStatsHolder will not appear for empty shards that have had no cache - * operations done on them yet. This might be changed in the future, by exposing a method to add empty nodes to the - * tree in CacheStatsHolder in the ICache interface. - * - * @opensearch.experimental + * An interface extended by DefaultCacheStatsHolder and NoopCacheStatsHolder. */ -public class CacheStatsHolder { - - // The list of permitted dimensions. Should be ordered from "outermost" to "innermost", as you would like to - // aggregate them in an API response. - private final List dimensionNames; - // A tree structure based on dimension values, which stores stats values in its leaf nodes. - // Non-leaf nodes have stats matching the sum of their children. - // We use a tree structure, rather than a map with concatenated keys, to save on memory usage. If there are many leaf - // nodes that share a parent, that parent's dimension value will only be stored once, not many times. - private final Node statsRoot; - // To avoid sync problems, obtain a lock before creating or removing nodes in the stats tree. - // No lock is needed to edit stats on existing nodes. - private final Lock lock = new ReentrantLock(); - - public CacheStatsHolder(List dimensionNames) { - this.dimensionNames = Collections.unmodifiableList(dimensionNames); - this.statsRoot = new Node("", true); // The root node has the empty string as its dimension value - } - - public List getDimensionNames() { - return dimensionNames; - } - - // For all these increment functions, the dimensions list comes from the key, and contains all dimensions present in dimensionNames. - // The order has to match the order given in dimensionNames. - public void incrementHits(List dimensionValues) { - internalIncrement(dimensionValues, Node::incrementHits, true); - } - - public void incrementMisses(List dimensionValues) { - internalIncrement(dimensionValues, Node::incrementMisses, true); - } - - public void incrementEvictions(List dimensionValues) { - internalIncrement(dimensionValues, Node::incrementEvictions, true); - } - - public void incrementSizeInBytes(List dimensionValues, long amountBytes) { - internalIncrement(dimensionValues, (node) -> node.incrementSizeInBytes(amountBytes), true); - } - - // For decrements, we should not create nodes if they are absent. This protects us from erroneously decrementing values for keys - // which have been entirely deleted, for example in an async removal listener. - public void decrementSizeInBytes(List dimensionValues, long amountBytes) { - internalIncrement(dimensionValues, (node) -> node.decrementSizeInBytes(amountBytes), false); - } - - public void incrementEntries(List dimensionValues) { - internalIncrement(dimensionValues, Node::incrementEntries, true); - } - - public void decrementEntries(List dimensionValues) { - internalIncrement(dimensionValues, Node::decrementEntries, false); - } - - /** - * Reset number of entries and memory size when all keys leave the cache, but don't reset hit/miss/eviction numbers. - * This is in line with the behavior of the existing API when caches are cleared. - */ - public void reset() { - resetHelper(statsRoot); - } - - private void resetHelper(Node current) { - current.resetSizeAndEntries(); - for (Node child : current.children.values()) { - resetHelper(child); - } - } - - public long count() { - // Include this here so caches don't have to create an entire CacheStats object to run count(). - return statsRoot.getEntries(); - } - - private void internalIncrement(List dimensionValues, Consumer adder, boolean createNodesIfAbsent) { - assert dimensionValues.size() == dimensionNames.size(); - // First try to increment without creating nodes - boolean didIncrement = internalIncrementHelper(dimensionValues, statsRoot, 0, adder, false); - // If we failed to increment, because nodes had to be created, obtain the lock and run again while creating nodes if needed - if (!didIncrement && createNodesIfAbsent) { - try { - lock.lock(); - internalIncrementHelper(dimensionValues, statsRoot, 0, adder, true); - } finally { - lock.unlock(); - } - } - } - - /** - * Use the incrementer function to increment/decrement a value in the stats for a set of dimensions. - * If createNodesIfAbsent is true, and there is no stats for this set of dimensions, create one. - * Returns true if the increment was applied, false if not. - */ - private boolean internalIncrementHelper( - List dimensionValues, - Node node, - int depth, // Pass in the depth to avoid having to slice the list for each node. - Consumer adder, - boolean createNodesIfAbsent - ) { - if (depth == dimensionValues.size()) { - // This is the leaf node we are trying to reach - adder.accept(node); - return true; - } - - Node child = node.getChild(dimensionValues.get(depth)); - if (child == null) { - if (createNodesIfAbsent) { - boolean createMapInChild = depth < dimensionValues.size() - 1; - child = node.createChild(dimensionValues.get(depth), createMapInChild); - } else { - return false; - } - } - if (internalIncrementHelper(dimensionValues, child, depth + 1, adder, createNodesIfAbsent)) { - // Function returns true if the next node down was incremented - adder.accept(node); - return true; - } - return false; - } - - /** - * Produce an immutable version of these stats. - */ - public ImmutableCacheStatsHolder getImmutableCacheStatsHolder() { - return new ImmutableCacheStatsHolder(statsRoot.snapshot(), dimensionNames); - } - - public void removeDimensions(List dimensionValues) { - assert dimensionValues.size() == dimensionNames.size() : "Must specify a value for every dimension when removing from StatsHolder"; - // As we are removing nodes from the tree, obtain the lock - lock.lock(); - try { - removeDimensionsHelper(dimensionValues, statsRoot, 0); - } finally { - lock.unlock(); - } - } - - // Returns a CacheStatsCounterSnapshot object for the stats to decrement if the removal happened, null otherwise. - private ImmutableCacheStats removeDimensionsHelper(List dimensionValues, Node node, int depth) { - if (depth == dimensionValues.size()) { - // Pass up a snapshot of the original stats to avoid issues when the original is decremented by other fn invocations - return node.getImmutableStats(); - } - Node child = node.getChild(dimensionValues.get(depth)); - if (child == null) { - return null; - } - ImmutableCacheStats statsToDecrement = removeDimensionsHelper(dimensionValues, child, depth + 1); - if (statsToDecrement != null) { - // The removal took place, decrement values and remove this node from its parent if it's now empty - node.decrementBySnapshot(statsToDecrement); - if (child.getChildren().isEmpty()) { - node.children.remove(child.getDimensionValue()); - } - } - return statsToDecrement; - } - - // pkg-private for testing - Node getStatsRoot() { - return statsRoot; - } - - static class Node { - private final String dimensionValue; - // Map from dimensionValue to the DimensionNode for that dimension value. - final Map children; - // The stats for this node. If a leaf node, corresponds to the stats for this combination of dimensions; if not, - // contains the sum of its children's stats. - private CacheStats stats; - - // Used for leaf nodes to avoid allocating many unnecessary maps - private static final Map EMPTY_CHILDREN_MAP = new HashMap<>(); - - Node(String dimensionValue, boolean createChildrenMap) { - this.dimensionValue = dimensionValue; - if (createChildrenMap) { - this.children = new ConcurrentHashMap<>(); - } else { - this.children = EMPTY_CHILDREN_MAP; - } - this.stats = new CacheStats(); - } - - public String getDimensionValue() { - return dimensionValue; - } - - protected Map getChildren() { - // We can safely iterate over ConcurrentHashMap without worrying about thread issues. - return children; - } - - // Functions for modifying internal CacheStatsCounter without callers having to be aware of CacheStatsCounter - - void incrementHits() { - this.stats.incrementHits(); - } - - void incrementMisses() { - this.stats.incrementMisses(); - } - - void incrementEvictions() { - this.stats.incrementEvictions(); - } - - void incrementSizeInBytes(long amountBytes) { - this.stats.incrementSizeInBytes(amountBytes); - } +public interface CacheStatsHolder { + void incrementHits(List dimensionValues); - void decrementSizeInBytes(long amountBytes) { - this.stats.decrementSizeInBytes(amountBytes); - } + void incrementMisses(List dimensionValues); - void incrementEntries() { - this.stats.incrementEntries(); - } + void incrementEvictions(List dimensionValues); - void decrementEntries() { - this.stats.decrementEntries(); - } + void incrementSizeInBytes(List dimensionValues, long amountBytes); - long getEntries() { - return this.stats.getEntries(); - } + void decrementSizeInBytes(List dimensionValues, long amountBytes); - ImmutableCacheStats getImmutableStats() { - return this.stats.immutableSnapshot(); - } + void incrementEntries(List dimensionValues); - void decrementBySnapshot(ImmutableCacheStats snapshot) { - this.stats.subtract(snapshot); - } + void decrementEntries(List dimensionValues); - void resetSizeAndEntries() { - this.stats.resetSizeAndEntries(); - } + void reset(); - Node getChild(String dimensionValue) { - return children.get(dimensionValue); - } + long count(); - Node createChild(String dimensionValue, boolean createMapInChild) { - return children.computeIfAbsent(dimensionValue, (key) -> new Node(dimensionValue, createMapInChild)); - } + void removeDimensions(List dimensionValues); - ImmutableCacheStatsHolder.Node snapshot() { - TreeMap snapshotChildren = null; - if (!children.isEmpty()) { - snapshotChildren = new TreeMap<>(); - for (Node child : children.values()) { - snapshotChildren.put(child.getDimensionValue(), child.snapshot()); - } - } - return new ImmutableCacheStatsHolder.Node(dimensionValue, snapshotChildren, getImmutableStats()); - } - } + ImmutableCacheStatsHolder getImmutableCacheStatsHolder(); } diff --git a/server/src/main/java/org/opensearch/common/cache/stats/DefaultCacheStatsHolder.java b/server/src/main/java/org/opensearch/common/cache/stats/DefaultCacheStatsHolder.java new file mode 100644 index 0000000000000..ad943e0b2ed1a --- /dev/null +++ b/server/src/main/java/org/opensearch/common/cache/stats/DefaultCacheStatsHolder.java @@ -0,0 +1,306 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.common.cache.stats; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Consumer; + +/** + * A class ICache implementations use to internally keep track of their stats across multiple dimensions. + * Not intended to be exposed outside the cache; for this, caches use getImmutableCacheStatsHolder() to create an immutable + * copy of the current state of the stats. + * Currently, in the IRC, the stats tracked in a CacheStatsHolder will not appear for empty shards that have had no cache + * operations done on them yet. This might be changed in the future, by exposing a method to add empty nodes to the + * tree in CacheStatsHolder in the ICache interface. + * + * @opensearch.experimental + */ +public class DefaultCacheStatsHolder implements CacheStatsHolder { + + // The list of permitted dimensions. Should be ordered from "outermost" to "innermost", as you would like to + // aggregate them in an API response. + private final List dimensionNames; + // A tree structure based on dimension values, which stores stats values in its leaf nodes. + // Non-leaf nodes have stats matching the sum of their children. + // We use a tree structure, rather than a map with concatenated keys, to save on memory usage. If there are many leaf + // nodes that share a parent, that parent's dimension value will only be stored once, not many times. + private final Node statsRoot; + // To avoid sync problems, obtain a lock before creating or removing nodes in the stats tree. + // No lock is needed to edit stats on existing nodes. + private final Lock lock = new ReentrantLock(); + + public DefaultCacheStatsHolder(List dimensionNames) { + this.dimensionNames = Collections.unmodifiableList(dimensionNames); + this.statsRoot = new Node("", true); // The root node has the empty string as its dimension value + } + + public List getDimensionNames() { + return dimensionNames; + } + + // For all these increment functions, the dimensions list comes from the key, and contains all dimensions present in dimensionNames. + // The order has to match the order given in dimensionNames. + @Override + public void incrementHits(List dimensionValues) { + internalIncrement(dimensionValues, Node::incrementHits, true); + } + + @Override + public void incrementMisses(List dimensionValues) { + internalIncrement(dimensionValues, Node::incrementMisses, true); + } + + @Override + public void incrementEvictions(List dimensionValues) { + internalIncrement(dimensionValues, Node::incrementEvictions, true); + } + + @Override + public void incrementSizeInBytes(List dimensionValues, long amountBytes) { + internalIncrement(dimensionValues, (node) -> node.incrementSizeInBytes(amountBytes), true); + } + + // For decrements, we should not create nodes if they are absent. This protects us from erroneously decrementing values for keys + // which have been entirely deleted, for example in an async removal listener. + @Override + public void decrementSizeInBytes(List dimensionValues, long amountBytes) { + internalIncrement(dimensionValues, (node) -> node.decrementSizeInBytes(amountBytes), false); + } + + @Override + public void incrementEntries(List dimensionValues) { + internalIncrement(dimensionValues, Node::incrementEntries, true); + } + + @Override + public void decrementEntries(List dimensionValues) { + internalIncrement(dimensionValues, Node::decrementEntries, false); + } + + /** + * Reset number of entries and memory size when all keys leave the cache, but don't reset hit/miss/eviction numbers. + * This is in line with the behavior of the existing API when caches are cleared. + */ + @Override + public void reset() { + resetHelper(statsRoot); + } + + private void resetHelper(Node current) { + current.resetSizeAndEntries(); + for (Node child : current.children.values()) { + resetHelper(child); + } + } + + @Override + public long count() { + // Include this here so caches don't have to create an entire CacheStats object to run count(). + return statsRoot.getEntries(); + } + + private void internalIncrement(List dimensionValues, Consumer adder, boolean createNodesIfAbsent) { + assert dimensionValues.size() == dimensionNames.size(); + // First try to increment without creating nodes + boolean didIncrement = internalIncrementHelper(dimensionValues, statsRoot, 0, adder, false); + // If we failed to increment, because nodes had to be created, obtain the lock and run again while creating nodes if needed + if (!didIncrement && createNodesIfAbsent) { + try { + lock.lock(); + internalIncrementHelper(dimensionValues, statsRoot, 0, adder, true); + } finally { + lock.unlock(); + } + } + } + + /** + * Use the incrementer function to increment/decrement a value in the stats for a set of dimensions. + * If createNodesIfAbsent is true, and there is no stats for this set of dimensions, create one. + * Returns true if the increment was applied, false if not. + */ + private boolean internalIncrementHelper( + List dimensionValues, + Node node, + int depth, // Pass in the depth to avoid having to slice the list for each node. + Consumer adder, + boolean createNodesIfAbsent + ) { + if (depth == dimensionValues.size()) { + // This is the leaf node we are trying to reach + adder.accept(node); + return true; + } + + Node child = node.getChild(dimensionValues.get(depth)); + if (child == null) { + if (createNodesIfAbsent) { + boolean createMapInChild = depth < dimensionValues.size() - 1; + child = node.createChild(dimensionValues.get(depth), createMapInChild); + } else { + return false; + } + } + if (internalIncrementHelper(dimensionValues, child, depth + 1, adder, createNodesIfAbsent)) { + // Function returns true if the next node down was incremented + adder.accept(node); + return true; + } + return false; + } + + /** + * Produce an immutable version of these stats. + */ + @Override + public ImmutableCacheStatsHolder getImmutableCacheStatsHolder() { + return new ImmutableCacheStatsHolder(statsRoot.snapshot(), dimensionNames); + } + + @Override + public void removeDimensions(List dimensionValues) { + assert dimensionValues.size() == dimensionNames.size() : "Must specify a value for every dimension when removing from StatsHolder"; + // As we are removing nodes from the tree, obtain the lock + lock.lock(); + try { + removeDimensionsHelper(dimensionValues, statsRoot, 0); + } finally { + lock.unlock(); + } + } + + // Returns a CacheStatsCounterSnapshot object for the stats to decrement if the removal happened, null otherwise. + private ImmutableCacheStats removeDimensionsHelper(List dimensionValues, Node node, int depth) { + if (depth == dimensionValues.size()) { + // Pass up a snapshot of the original stats to avoid issues when the original is decremented by other fn invocations + return node.getImmutableStats(); + } + Node child = node.getChild(dimensionValues.get(depth)); + if (child == null) { + return null; + } + ImmutableCacheStats statsToDecrement = removeDimensionsHelper(dimensionValues, child, depth + 1); + if (statsToDecrement != null) { + // The removal took place, decrement values and remove this node from its parent if it's now empty + node.decrementBySnapshot(statsToDecrement); + if (child.getChildren().isEmpty()) { + node.children.remove(child.getDimensionValue()); + } + } + return statsToDecrement; + } + + // pkg-private for testing + Node getStatsRoot() { + return statsRoot; + } + + static class Node { + private final String dimensionValue; + // Map from dimensionValue to the DimensionNode for that dimension value. + final Map children; + // The stats for this node. If a leaf node, corresponds to the stats for this combination of dimensions; if not, + // contains the sum of its children's stats. + private CacheStats stats; + + // Used for leaf nodes to avoid allocating many unnecessary maps + private static final Map EMPTY_CHILDREN_MAP = new HashMap<>(); + + Node(String dimensionValue, boolean createChildrenMap) { + this.dimensionValue = dimensionValue; + if (createChildrenMap) { + this.children = new ConcurrentHashMap<>(); + } else { + this.children = EMPTY_CHILDREN_MAP; + } + this.stats = new CacheStats(); + } + + public String getDimensionValue() { + return dimensionValue; + } + + protected Map getChildren() { + // We can safely iterate over ConcurrentHashMap without worrying about thread issues. + return children; + } + + // Functions for modifying internal CacheStatsCounter without callers having to be aware of CacheStatsCounter + + void incrementHits() { + this.stats.incrementHits(); + } + + void incrementMisses() { + this.stats.incrementMisses(); + } + + void incrementEvictions() { + this.stats.incrementEvictions(); + } + + void incrementSizeInBytes(long amountBytes) { + this.stats.incrementSizeInBytes(amountBytes); + } + + void decrementSizeInBytes(long amountBytes) { + this.stats.decrementSizeInBytes(amountBytes); + } + + void incrementEntries() { + this.stats.incrementEntries(); + } + + void decrementEntries() { + this.stats.decrementEntries(); + } + + long getEntries() { + return this.stats.getEntries(); + } + + ImmutableCacheStats getImmutableStats() { + return this.stats.immutableSnapshot(); + } + + void decrementBySnapshot(ImmutableCacheStats snapshot) { + this.stats.subtract(snapshot); + } + + void resetSizeAndEntries() { + this.stats.resetSizeAndEntries(); + } + + Node getChild(String dimensionValue) { + return children.get(dimensionValue); + } + + Node createChild(String dimensionValue, boolean createMapInChild) { + return children.computeIfAbsent(dimensionValue, (key) -> new Node(dimensionValue, createMapInChild)); + } + + ImmutableCacheStatsHolder.Node snapshot() { + TreeMap snapshotChildren = null; + if (!children.isEmpty()) { + snapshotChildren = new TreeMap<>(); + for (Node child : children.values()) { + snapshotChildren.put(child.getDimensionValue(), child.snapshot()); + } + } + return new ImmutableCacheStatsHolder.Node(dimensionValue, snapshotChildren, getImmutableStats()); + } + } +} diff --git a/server/src/main/java/org/opensearch/common/cache/stats/NoopCacheStatsHolder.java b/server/src/main/java/org/opensearch/common/cache/stats/NoopCacheStatsHolder.java new file mode 100644 index 0000000000000..b7debbd8a8eab --- /dev/null +++ b/server/src/main/java/org/opensearch/common/cache/stats/NoopCacheStatsHolder.java @@ -0,0 +1,68 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.common.cache.stats; + +import java.util.List; + +/** + * A dummy version of CacheStatsHolder, which cache implementations use when FeatureFlags.PLUGGABLE_CACHES is false. + * Returns all-zero stats when calling getImmutableCacheStatsHolder(). Always returns 0 for count(). + * A singleton instance is used for memory purposes. + */ +public class NoopCacheStatsHolder implements CacheStatsHolder { + private static final NoopCacheStatsHolder singletonInstance = new NoopCacheStatsHolder(); + private static final ImmutableCacheStatsHolder immutableCacheStatsHolder; + static { + ImmutableCacheStatsHolder.Node dummyNode = new ImmutableCacheStatsHolder.Node("", null, new ImmutableCacheStats(0, 0, 0, 0, 0)); + immutableCacheStatsHolder = new ImmutableCacheStatsHolder(dummyNode, List.of()); + } + + private NoopCacheStatsHolder() {} + + public static NoopCacheStatsHolder getInstance() { + return singletonInstance; + } + + @Override + public void incrementHits(List dimensionValues) {} + + @Override + public void incrementMisses(List dimensionValues) {} + + @Override + public void incrementEvictions(List dimensionValues) {} + + @Override + public void incrementSizeInBytes(List dimensionValues, long amountBytes) {} + + @Override + public void decrementSizeInBytes(List dimensionValues, long amountBytes) {} + + @Override + public void incrementEntries(List dimensionValues) {} + + @Override + public void decrementEntries(List dimensionValues) {} + + @Override + public void reset() {} + + @Override + public long count() { + return 0; + } + + @Override + public void removeDimensions(List dimensionValues) {} + + @Override + public ImmutableCacheStatsHolder getImmutableCacheStatsHolder() { + return immutableCacheStatsHolder; + } +} diff --git a/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java b/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java index 29e5667c9f27d..35c951e240a3a 100644 --- a/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java +++ b/server/src/main/java/org/opensearch/common/cache/store/OpenSearchOnHeapCache.java @@ -19,7 +19,9 @@ import org.opensearch.common.cache.RemovalReason; import org.opensearch.common.cache.settings.CacheSettings; import org.opensearch.common.cache.stats.CacheStatsHolder; +import org.opensearch.common.cache.stats.DefaultCacheStatsHolder; import org.opensearch.common.cache.stats.ImmutableCacheStatsHolder; +import org.opensearch.common.cache.stats.NoopCacheStatsHolder; import org.opensearch.common.cache.store.builders.ICacheBuilder; import org.opensearch.common.cache.store.config.CacheConfig; import org.opensearch.common.cache.store.settings.OpenSearchOnHeapCacheSettings; @@ -62,7 +64,13 @@ public OpenSearchOnHeapCache(Builder builder) { } cache = cacheBuilder.build(); this.dimensionNames = Objects.requireNonNull(builder.dimensionNames, "Dimension names can't be null"); - this.cacheStatsHolder = new CacheStatsHolder(dimensionNames); + // Use noop stats when pluggable caching is off + boolean useNoopStats = !FeatureFlags.PLUGGABLE_CACHE_SETTING.get(builder.getSettings()); + if (useNoopStats) { + this.cacheStatsHolder = NoopCacheStatsHolder.getInstance(); + } else { + this.cacheStatsHolder = new DefaultCacheStatsHolder(dimensionNames); + } this.removalListener = builder.getRemovalListener(); this.weigher = builder.getWeigher(); } @@ -121,7 +129,7 @@ public Iterable> keys() { @Override public long count() { - return cacheStatsHolder.count(); + return cache.count(); } @Override @@ -164,6 +172,7 @@ public ICache create(CacheConfig config, CacheType cacheType, Map> settingList = OpenSearchOnHeapCacheSettings.getSettingListForCacheType(cacheType); Settings settings = config.getSettings(); ICacheBuilder builder = new Builder().setDimensionNames(config.getDimensionNames()) + .setSettings(config.getSettings()) .setMaximumWeightInBytes(((ByteSizeValue) settingList.get(MAXIMUM_SIZE_IN_BYTES_KEY).get(settings)).getBytes()) .setExpireAfterAccess(((TimeValue) settingList.get(EXPIRE_AFTER_ACCESS_KEY).get(settings))) .setWeigher(config.getWeigher()) diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java index 105239b2c351b..f9a9c1830e1ad 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java @@ -825,13 +825,6 @@ long count() { return cache.count(); } - /** - * Returns the current size in bytes of the cache - */ - long getSizeInBytes() { - return cache.stats().getTotalSizeInBytes(); - } - /** * Returns the current cache stats. Pkg-private for testing. */ diff --git a/server/src/test/java/org/opensearch/common/cache/stats/CacheStatsHolderTests.java b/server/src/test/java/org/opensearch/common/cache/stats/DefaultCacheStatsHolderTests.java similarity index 85% rename from server/src/test/java/org/opensearch/common/cache/stats/CacheStatsHolderTests.java rename to server/src/test/java/org/opensearch/common/cache/stats/DefaultCacheStatsHolderTests.java index 390cd4d601a4b..fe12673bb9f6a 100644 --- a/server/src/test/java/org/opensearch/common/cache/stats/CacheStatsHolderTests.java +++ b/server/src/test/java/org/opensearch/common/cache/stats/DefaultCacheStatsHolderTests.java @@ -21,18 +21,23 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; -public class CacheStatsHolderTests extends OpenSearchTestCase { +public class DefaultCacheStatsHolderTests extends OpenSearchTestCase { public void testAddAndGet() throws Exception { List dimensionNames = List.of("dim1", "dim2", "dim3", "dim4"); - CacheStatsHolder cacheStatsHolder = new CacheStatsHolder(dimensionNames); - Map> usedDimensionValues = CacheStatsHolderTests.getUsedDimensionValues(cacheStatsHolder, 10); - Map, CacheStats> expected = CacheStatsHolderTests.populateStats(cacheStatsHolder, usedDimensionValues, 1000, 10); + DefaultCacheStatsHolder cacheStatsHolder = new DefaultCacheStatsHolder(dimensionNames); + Map> usedDimensionValues = DefaultCacheStatsHolderTests.getUsedDimensionValues(cacheStatsHolder, 10); + Map, CacheStats> expected = DefaultCacheStatsHolderTests.populateStats( + cacheStatsHolder, + usedDimensionValues, + 1000, + 10 + ); // test the value in the map is as expected for each distinct combination of values for (List dimensionValues : expected.keySet()) { CacheStats expectedCounter = expected.get(dimensionValues); - ImmutableCacheStats actualStatsHolder = CacheStatsHolderTests.getNode(dimensionValues, cacheStatsHolder.getStatsRoot()) + ImmutableCacheStats actualStatsHolder = DefaultCacheStatsHolderTests.getNode(dimensionValues, cacheStatsHolder.getStatsRoot()) .getImmutableStats(); ImmutableCacheStats actualCacheStats = getNode(dimensionValues, cacheStatsHolder.getStatsRoot()).getImmutableStats(); @@ -53,7 +58,7 @@ public void testAddAndGet() throws Exception { public void testReset() throws Exception { List dimensionNames = List.of("dim1", "dim2"); - CacheStatsHolder cacheStatsHolder = new CacheStatsHolder(dimensionNames); + DefaultCacheStatsHolder cacheStatsHolder = new DefaultCacheStatsHolder(dimensionNames); Map> usedDimensionValues = getUsedDimensionValues(cacheStatsHolder, 10); Map, CacheStats> expected = populateStats(cacheStatsHolder, usedDimensionValues, 100, 10); @@ -64,7 +69,7 @@ public void testReset() throws Exception { originalCounter.sizeInBytes = new CounterMetric(); originalCounter.entries = new CounterMetric(); - CacheStatsHolder.Node node = getNode(dimensionValues, cacheStatsHolder.getStatsRoot()); + DefaultCacheStatsHolder.Node node = getNode(dimensionValues, cacheStatsHolder.getStatsRoot()); ImmutableCacheStats actual = node.getImmutableStats(); assertEquals(originalCounter.immutableSnapshot(), actual); } @@ -72,7 +77,7 @@ public void testReset() throws Exception { public void testDropStatsForDimensions() throws Exception { List dimensionNames = List.of("dim1", "dim2"); - CacheStatsHolder cacheStatsHolder = new CacheStatsHolder(dimensionNames); + DefaultCacheStatsHolder cacheStatsHolder = new DefaultCacheStatsHolder(dimensionNames); // Create stats for the following dimension sets List> populatedStats = List.of(List.of("A1", "B1"), List.of("A2", "B2"), List.of("A2", "B3")); @@ -108,7 +113,7 @@ public void testDropStatsForDimensions() throws Exception { public void testCount() throws Exception { List dimensionNames = List.of("dim1", "dim2"); - CacheStatsHolder cacheStatsHolder = new CacheStatsHolder(dimensionNames); + DefaultCacheStatsHolder cacheStatsHolder = new DefaultCacheStatsHolder(dimensionNames); Map> usedDimensionValues = getUsedDimensionValues(cacheStatsHolder, 10); Map, CacheStats> expected = populateStats(cacheStatsHolder, usedDimensionValues, 100, 10); @@ -121,7 +126,7 @@ public void testCount() throws Exception { public void testConcurrentRemoval() throws Exception { List dimensionNames = List.of("dim1", "dim2"); - CacheStatsHolder cacheStatsHolder = new CacheStatsHolder(dimensionNames); + DefaultCacheStatsHolder cacheStatsHolder = new DefaultCacheStatsHolder(dimensionNames); // Create stats for the following dimension sets List> populatedStats = List.of(List.of("A1", "B1"), List.of("A2", "B2"), List.of("A2", "B3")); @@ -169,8 +174,8 @@ public void testConcurrentRemoval() throws Exception { * Returns the node found by following these dimension values down from the root node. * Returns null if no such node exists. */ - static CacheStatsHolder.Node getNode(List dimensionValues, CacheStatsHolder.Node root) { - CacheStatsHolder.Node current = root; + static DefaultCacheStatsHolder.Node getNode(List dimensionValues, DefaultCacheStatsHolder.Node root) { + DefaultCacheStatsHolder.Node current = root; for (String dimensionValue : dimensionValues) { current = current.getChildren().get(dimensionValue); if (current == null) { @@ -181,7 +186,7 @@ static CacheStatsHolder.Node getNode(List dimensionValues, CacheStatsHol } static Map, CacheStats> populateStats( - CacheStatsHolder cacheStatsHolder, + DefaultCacheStatsHolder cacheStatsHolder, Map> usedDimensionValues, int numDistinctValuePairs, int numRepetitionsPerValue @@ -211,7 +216,7 @@ static Map, CacheStats> populateStats( expected.get(dimensions).evictions.inc(statsToInc.getEvictions()); expected.get(dimensions).sizeInBytes.inc(statsToInc.getSizeInBytes()); expected.get(dimensions).entries.inc(statsToInc.getEntries()); - CacheStatsHolderTests.populateStatsHolderFromStatsValueMap(cacheStatsHolder, Map.of(dimensions, statsToInc)); + DefaultCacheStatsHolderTests.populateStatsHolderFromStatsValueMap(cacheStatsHolder, Map.of(dimensions, statsToInc)); } countDownLatch.countDown(); }); @@ -240,7 +245,7 @@ private static List getRandomDimList( return result; } - static Map> getUsedDimensionValues(CacheStatsHolder cacheStatsHolder, int numValuesPerDim) { + static Map> getUsedDimensionValues(DefaultCacheStatsHolder cacheStatsHolder, int numValuesPerDim) { Map> usedDimensionValues = new HashMap<>(); for (int i = 0; i < cacheStatsHolder.getDimensionNames().size(); i++) { List values = new ArrayList<>(); @@ -252,20 +257,20 @@ static Map> getUsedDimensionValues(CacheStatsHolder cacheSt return usedDimensionValues; } - private void assertSumOfChildrenStats(CacheStatsHolder.Node current) { + private void assertSumOfChildrenStats(DefaultCacheStatsHolder.Node current) { if (!current.children.isEmpty()) { CacheStats expectedTotal = new CacheStats(); - for (CacheStatsHolder.Node child : current.children.values()) { + for (DefaultCacheStatsHolder.Node child : current.children.values()) { expectedTotal.add(child.getImmutableStats()); } assertEquals(expectedTotal.immutableSnapshot(), current.getImmutableStats()); - for (CacheStatsHolder.Node child : current.children.values()) { + for (DefaultCacheStatsHolder.Node child : current.children.values()) { assertSumOfChildrenStats(child); } } } - static void populateStatsHolderFromStatsValueMap(CacheStatsHolder cacheStatsHolder, Map, CacheStats> statsMap) { + static void populateStatsHolderFromStatsValueMap(DefaultCacheStatsHolder cacheStatsHolder, Map, CacheStats> statsMap) { for (Map.Entry, CacheStats> entry : statsMap.entrySet()) { CacheStats stats = entry.getValue(); List dims = entry.getKey(); diff --git a/server/src/test/java/org/opensearch/common/cache/stats/ImmutableCacheStatsHolderTests.java b/server/src/test/java/org/opensearch/common/cache/stats/ImmutableCacheStatsHolderTests.java index 933b8abd6e392..5a4511fa654dd 100644 --- a/server/src/test/java/org/opensearch/common/cache/stats/ImmutableCacheStatsHolderTests.java +++ b/server/src/test/java/org/opensearch/common/cache/stats/ImmutableCacheStatsHolderTests.java @@ -17,17 +17,24 @@ public class ImmutableCacheStatsHolderTests extends OpenSearchTestCase { public void testGet() throws Exception { List dimensionNames = List.of("dim1", "dim2", "dim3", "dim4"); - CacheStatsHolder cacheStatsHolder = new CacheStatsHolder(dimensionNames); - Map> usedDimensionValues = CacheStatsHolderTests.getUsedDimensionValues(cacheStatsHolder, 10); - Map, CacheStats> expected = CacheStatsHolderTests.populateStats(cacheStatsHolder, usedDimensionValues, 1000, 10); + DefaultCacheStatsHolder cacheStatsHolder = new DefaultCacheStatsHolder(dimensionNames); + Map> usedDimensionValues = DefaultCacheStatsHolderTests.getUsedDimensionValues(cacheStatsHolder, 10); + Map, CacheStats> expected = DefaultCacheStatsHolderTests.populateStats( + cacheStatsHolder, + usedDimensionValues, + 1000, + 10 + ); ImmutableCacheStatsHolder stats = cacheStatsHolder.getImmutableCacheStatsHolder(); // test the value in the map is as expected for each distinct combination of values for (List dimensionValues : expected.keySet()) { CacheStats expectedCounter = expected.get(dimensionValues); - ImmutableCacheStats actualCacheStatsHolder = CacheStatsHolderTests.getNode(dimensionValues, cacheStatsHolder.getStatsRoot()) - .getImmutableStats(); + ImmutableCacheStats actualCacheStatsHolder = DefaultCacheStatsHolderTests.getNode( + dimensionValues, + cacheStatsHolder.getStatsRoot() + ).getImmutableStats(); ImmutableCacheStats actualImmutableCacheStatsHolder = getNode(dimensionValues, stats.getStatsRoot()).getStats(); assertEquals(expectedCounter.immutableSnapshot(), actualCacheStatsHolder); @@ -52,9 +59,9 @@ public void testGet() throws Exception { public void testEmptyDimsList() throws Exception { // If the dimension list is empty, the tree should have only the root node containing the total stats. - CacheStatsHolder cacheStatsHolder = new CacheStatsHolder(List.of()); - Map> usedDimensionValues = CacheStatsHolderTests.getUsedDimensionValues(cacheStatsHolder, 100); - CacheStatsHolderTests.populateStats(cacheStatsHolder, usedDimensionValues, 10, 100); + DefaultCacheStatsHolder cacheStatsHolder = new DefaultCacheStatsHolder(List.of()); + Map> usedDimensionValues = DefaultCacheStatsHolderTests.getUsedDimensionValues(cacheStatsHolder, 100); + DefaultCacheStatsHolderTests.populateStats(cacheStatsHolder, usedDimensionValues, 10, 100); ImmutableCacheStatsHolder stats = cacheStatsHolder.getImmutableCacheStatsHolder(); ImmutableCacheStatsHolder.Node statsRoot = stats.getStatsRoot(); diff --git a/server/src/test/java/org/opensearch/common/cache/store/OpenSearchOnHeapCacheTests.java b/server/src/test/java/org/opensearch/common/cache/store/OpenSearchOnHeapCacheTests.java index 008dc7c2e0902..00dbf43bc37be 100644 --- a/server/src/test/java/org/opensearch/common/cache/store/OpenSearchOnHeapCacheTests.java +++ b/server/src/test/java/org/opensearch/common/cache/store/OpenSearchOnHeapCacheTests.java @@ -16,10 +16,12 @@ import org.opensearch.common.cache.RemovalListener; import org.opensearch.common.cache.RemovalNotification; import org.opensearch.common.cache.stats.ImmutableCacheStats; +import org.opensearch.common.cache.stats.ImmutableCacheStatsHolder; import org.opensearch.common.cache.store.config.CacheConfig; import org.opensearch.common.cache.store.settings.OpenSearchOnHeapCacheSettings; import org.opensearch.common.metrics.CounterMetric; import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.test.OpenSearchTestCase; import java.util.ArrayList; @@ -37,7 +39,9 @@ public void testStats() throws Exception { MockRemovalListener listener = new MockRemovalListener<>(); int maxKeys = between(10, 50); int numEvicted = between(10, 20); - OpenSearchOnHeapCache cache = getCache(maxKeys, listener); + OpenSearchOnHeapCache cache = getCache(maxKeys, listener, true); + + // When the pluggable caches setting is on, we should get stats as expected from cache.stats(). List> keysAdded = new ArrayList<>(); int numAdded = maxKeys + numEvicted; @@ -77,7 +81,34 @@ public void testStats() throws Exception { } } - private OpenSearchOnHeapCache getCache(int maxSizeKeys, MockRemovalListener listener) { + public void testStatsWithoutPluggableCaches() throws Exception { + // When the pluggable caches setting is off, we should get all-zero stats from cache.stats(), but count() should still work. + MockRemovalListener listener = new MockRemovalListener<>(); + int maxKeys = between(10, 50); + int numEvicted = between(10, 20); + OpenSearchOnHeapCache cache = getCache(maxKeys, listener, false); + + List> keysAdded = new ArrayList<>(); + int numAdded = maxKeys + numEvicted; + for (int i = 0; i < numAdded; i++) { + ICacheKey key = getICacheKey(UUID.randomUUID().toString()); + keysAdded.add(key); + cache.computeIfAbsent(key, getLoadAwareCacheLoader()); + + assertEquals(Math.min(maxKeys, i + 1), cache.count()); + assertZeroStats(cache.stats()); + } + } + + private void assertZeroStats(ImmutableCacheStatsHolder stats) { + assertEquals(new ImmutableCacheStats(0, 0, 0, 0, 0), stats.getTotalStats()); + } + + private OpenSearchOnHeapCache getCache( + int maxSizeKeys, + MockRemovalListener listener, + boolean pluggableCachesSetting + ) { ICache.Factory onHeapCacheFactory = new OpenSearchOnHeapCache.OpenSearchOnHeapCacheFactory(); Settings settings = Settings.builder() .put( @@ -86,6 +117,7 @@ private OpenSearchOnHeapCache getCache(int maxSizeKeys, MockRemo .getKey(), maxSizeKeys * keyValueSize + "b" ) + .put(FeatureFlags.PLUGGABLE_CACHE, pluggableCachesSetting) .build(); CacheConfig cacheConfig = new CacheConfig.Builder().setKeyType(String.class) @@ -102,7 +134,7 @@ private OpenSearchOnHeapCache getCache(int maxSizeKeys, MockRemo public void testInvalidateWithDropDimensions() throws Exception { MockRemovalListener listener = new MockRemovalListener<>(); int maxKeys = 50; - OpenSearchOnHeapCache cache = getCache(maxKeys, listener); + OpenSearchOnHeapCache cache = getCache(maxKeys, listener, true); List> keysAdded = new ArrayList<>(); diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index 71eb30fca2385..fc306f7c595d6 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -51,6 +51,7 @@ import org.opensearch.common.cache.RemovalReason; import org.opensearch.common.cache.module.CacheModule; import org.opensearch.common.cache.stats.ImmutableCacheStats; +import org.opensearch.common.cache.stats.ImmutableCacheStatsHolder; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.lucene.index.OpenSearchDirectoryReader; import org.opensearch.common.settings.Settings; @@ -812,8 +813,12 @@ public void testClosingIndexWipesStats() throws Exception { assertNotNull(indexToKeep.getShard(i)); assertNotNull(indexToClose.getShard(i)); } + threadPool = getThreadPool(); - Settings settings = Settings.builder().put(INDICES_REQUEST_CACHE_STALENESS_THRESHOLD_SETTING.getKey(), "0.001%").build(); + Settings settings = Settings.builder() + .put(INDICES_REQUEST_CACHE_STALENESS_THRESHOLD_SETTING.getKey(), "0.001%") + .put(FeatureFlags.PLUGGABLE_CACHE, true) + .build(); cache = new IndicesRequestCache(settings, (shardId -> { IndexService indexService = null; try { @@ -868,6 +873,7 @@ public void testClosingIndexWipesStats() throws Exception { ShardId shardId = indexService.getShard(i).shardId(); List dimensionValues = List.of(shardId.getIndexName(), shardId.toString()); initialDimensionValues.add(dimensionValues); + ImmutableCacheStatsHolder holder = cache.stats(); ImmutableCacheStats snapshot = cache.stats().getStatsForDimensionValues(dimensionValues); assertNotNull(snapshot); // check the values are not empty by confirming entries != 0, this should always be true since the missed value is loaded @@ -954,13 +960,14 @@ public void testEviction() throws Exception { assertEquals("foo", value1.streamInput().readString()); BytesReference value2 = cache.getOrCompute(getEntity(indexShard), getLoader(secondReader), secondReader, getTermBytes()); assertEquals("bar", value2.streamInput().readString()); - size = new ByteSizeValue(cache.getSizeInBytes()); + size = indexShard.requestCache().stats().getMemorySize(); // Value from old API IOUtils.close(reader, secondReader, writer, dir, cache); } indexShard = createIndex("test1").getShard(0); IndicesRequestCache cache = new IndicesRequestCache( - // Add 5 instead of 1; the key size now depends on the length of dimension names and values so there's more variation - Settings.builder().put(IndicesRequestCache.INDICES_CACHE_QUERY_SIZE.getKey(), size.getBytes() + 5 + "b").build(), + // TODO: Add wiggle room to max size to allow for overhead of ICacheKey. This can be removed once API PR goes in, as it updates + // the old API to account for the ICacheKey overhead. + Settings.builder().put(IndicesRequestCache.INDICES_CACHE_QUERY_SIZE.getKey(), (int) (size.getBytes() * 1.2) + "b").build(), (shardId -> Optional.of(new IndicesService.IndexShardCacheEntity(indexShard))), new CacheModule(new ArrayList<>(), Settings.EMPTY).getCacheService(), threadPool,