From e6dca8dd1364042cfa45df38802b6e150186d326 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Fri, 29 Mar 2024 09:51:16 -0700 Subject: [PATCH] changed statsholder key to contain whole dimension Signed-off-by: Peter Alfonsi --- .../cache/stats/MultiDimensionCacheStats.java | 8 +++--- .../common/cache/stats/StatsHolder.java | 28 +++++++++++-------- .../stats/MultiDimensionCacheStatsTests.java | 8 ++++-- .../common/cache/stats/StatsHolderTests.java | 26 +++++++++++++---- 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/server/src/main/java/org/opensearch/common/cache/stats/MultiDimensionCacheStats.java b/server/src/main/java/org/opensearch/common/cache/stats/MultiDimensionCacheStats.java index 55a16b575d3ea..a5ff8b6c5cca1 100644 --- a/server/src/main/java/org/opensearch/common/cache/stats/MultiDimensionCacheStats.java +++ b/server/src/main/java/org/opensearch/common/cache/stats/MultiDimensionCacheStats.java @@ -44,7 +44,7 @@ public MultiDimensionCacheStats(Map snapshot, public MultiDimensionCacheStats(StreamInput in) throws IOException { this.dimensionNames = List.of(in.readStringArray()); this.snapshot = in.readMap( - i -> new StatsHolder.Key(List.of(i.readArray(StreamInput::readString, String[]::new))), + i -> new StatsHolder.Key(List.of(i.readArray(CacheStatsDimension::new, CacheStatsDimension[]::new))), CounterSnapshot::new ); } @@ -54,7 +54,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeStringArray(dimensionNames.toArray(new String[0])); out.writeMap( snapshot, - (o, key) -> o.writeArray((o1, dimValue) -> o1.writeString((String) dimValue), key.dimensionValues.toArray()), + (o, key) -> o.writeArray((o1, dim) -> ((CacheStatsDimension) dim).writeTo(o1), key.dimensions.toArray()), (o, snapshot) -> snapshot.writeTo(o) ); } @@ -158,9 +158,9 @@ DimensionNode aggregateByLevels(List levels) { DimensionNode root = new DimensionNode(null); for (Map.Entry entry : snapshot.entrySet()) { List levelValues = new ArrayList<>(); // This key's relevant dimension values, which match the levels - List keyDimensionValues = entry.getKey().dimensionValues; + List keyDimensions = entry.getKey().dimensions; for (int levelPosition : levelPositions) { - levelValues.add(keyDimensionValues.get(levelPosition)); + levelValues.add(keyDimensions.get(levelPosition).dimensionValue); } DimensionNode leafNode = root.getNode(levelValues); leafNode.addSnapshot(entry.getValue()); diff --git a/server/src/main/java/org/opensearch/common/cache/stats/StatsHolder.java b/server/src/main/java/org/opensearch/common/cache/stats/StatsHolder.java index 31b0fe37751db..11e4a4c622525 100644 --- a/server/src/main/java/org/opensearch/common/cache/stats/StatsHolder.java +++ b/server/src/main/java/org/opensearch/common/cache/stats/StatsHolder.java @@ -137,17 +137,17 @@ private CacheStatsCounter internalGetStats(List dimensions) * Get a valid key from an unordered list of dimensions. */ private Key getKey(List dims) { - return new Key(getOrderedDimensionValues(dims, dimensionNames)); + return new Key(getOrderedDimensions(dims, dimensionNames)); } // Get a list of dimension values, ordered according to dimensionNames, from the possibly differently-ordered dimensions passed in. // Public and static for testing purposes. - public static List getOrderedDimensionValues(List dimensions, List dimensionNames) { - List result = new ArrayList<>(); + public static List getOrderedDimensions(List dimensions, List dimensionNames) { + List result = new ArrayList<>(); for (String dimensionName : dimensionNames) { for (CacheStatsDimension dim : dimensions) { if (dim.dimensionName.equals(dimensionName)) { - result.add(dim.dimensionValue); + result.add(dim); } } } @@ -183,13 +183,17 @@ public void removeDimensions(List dims) { } } + /** + * Check if the Key contains all the dimensions in dims, matching both dimension name and value. + */ boolean keyContainsAllDimensions(Key key, List dims) { for (CacheStatsDimension dim : dims) { int dimensionPosition = dimensionNames.indexOf(dim.dimensionName); if (dimensionPosition == -1) { throw new IllegalArgumentException("Unrecognized dimension: " + dim.dimensionName + " = " + dim.dimensionValue); } - if (!key.dimensionValues.get(dimensionPosition).equals(dim.dimensionValue)) { + String keyDimensionValue = key.dimensions.get(dimensionPosition).dimensionValue; + if (!keyDimensionValue.equals(dim.dimensionValue)) { return false; } } @@ -200,14 +204,14 @@ boolean keyContainsAllDimensions(Key key, List dims) { * Unmodifiable wrapper over a list of dimension values, ordered according to dimensionNames. Pkg-private for testing. */ public static class Key { - final List dimensionValues; // The dimensions must be ordered + final List dimensions; // The dimensions must be ordered - public Key(List dimensionValues) { - this.dimensionValues = Collections.unmodifiableList(dimensionValues); + public Key(List dimensions) { + this.dimensions = Collections.unmodifiableList(dimensions); } - public List getDimensionValues() { - return dimensionValues; + public List getDimensions() { + return dimensions; } @Override @@ -222,12 +226,12 @@ public boolean equals(Object o) { return false; } Key other = (Key) o; - return this.dimensionValues.equals(other.dimensionValues); + return this.dimensions.equals(other.dimensions); } @Override public int hashCode() { - return this.dimensionValues.hashCode(); + return this.dimensions.hashCode(); } } } diff --git a/server/src/test/java/org/opensearch/common/cache/stats/MultiDimensionCacheStatsTests.java b/server/src/test/java/org/opensearch/common/cache/stats/MultiDimensionCacheStatsTests.java index 99c9f13c80a4d..6d9361e10aed2 100644 --- a/server/src/test/java/org/opensearch/common/cache/stats/MultiDimensionCacheStatsTests.java +++ b/server/src/test/java/org/opensearch/common/cache/stats/MultiDimensionCacheStatsTests.java @@ -61,7 +61,7 @@ public void testAddAndGet() throws Exception { // test the value in the map is as expected for each distinct combination of values for (List dims : expected.keySet()) { CacheStatsCounter expectedCounter = expected.get(dims); - StatsHolder.Key key = new StatsHolder.Key(StatsHolder.getOrderedDimensionValues(dims, dimensionNames)); + StatsHolder.Key key = new StatsHolder.Key(StatsHolder.getOrderedDimensions(dims, dimensionNames)); CounterSnapshot actual = stats.snapshot.get(key); assertEquals(expectedCounter.snapshot(), actual); @@ -134,10 +134,14 @@ public void testAggregateBySomeDimensions() throws Exception { for (Map.Entry, MultiDimensionCacheStats.DimensionNode> aggEntry : aggregatedLeafNodes.entrySet()) { CacheStatsCounter expectedCounter = new CacheStatsCounter(); for (List expectedDims : expected.keySet()) { - List orderedDimValues = StatsHolder.getOrderedDimensionValues( + List orderedDims = StatsHolder.getOrderedDimensions( new ArrayList<>(expectedDims), dimensionNames ); + List orderedDimValues = new ArrayList<>(); + for (CacheStatsDimension dim : orderedDims) { + orderedDimValues.add(dim.dimensionValue); + } if (orderedDimValues.containsAll(aggEntry.getKey())) { expectedCounter.add(expected.get(expectedDims)); } diff --git a/server/src/test/java/org/opensearch/common/cache/stats/StatsHolderTests.java b/server/src/test/java/org/opensearch/common/cache/stats/StatsHolderTests.java index 79e652afb9dbe..5bdbb6e2ab0e9 100644 --- a/server/src/test/java/org/opensearch/common/cache/stats/StatsHolderTests.java +++ b/server/src/test/java/org/opensearch/common/cache/stats/StatsHolderTests.java @@ -26,10 +26,18 @@ public class StatsHolderTests extends OpenSearchTestCase { // in MultiDimensionCacheStatsTests.java. public void testKeyEquality() throws Exception { - List dims1 = List.of("1", "2", "3"); + List dims1 = List.of( + new CacheStatsDimension("A", "1"), + new CacheStatsDimension("B", "2"), + new CacheStatsDimension("C", "3") + ); StatsHolder.Key key1 = new StatsHolder.Key(dims1); - List dims2 = List.of("1", "2", "3"); + List dims2 = List.of( + new CacheStatsDimension("A", "1"), + new CacheStatsDimension("B", "2"), + new CacheStatsDimension("C", "3") + ); StatsHolder.Key key2 = new StatsHolder.Key(dims2); assertEquals(key1, key2); @@ -49,7 +57,7 @@ public void testReset() throws Exception { originalCounter.sizeInBytes = new CounterMetric(); originalCounter.entries = new CounterMetric(); - StatsHolder.Key key = new StatsHolder.Key(StatsHolder.getOrderedDimensionValues(dims, dimensionNames)); + StatsHolder.Key key = new StatsHolder.Key(StatsHolder.getOrderedDimensions(dims, dimensionNames)); CacheStatsCounter actual = statsHolder.getStatsMap().get(key); assertEquals(originalCounter, actual); } @@ -68,8 +76,16 @@ public void testKeyContainsAllDimensions() throws Exception { List dims = List.of(new CacheStatsDimension("dim1", "A"), new CacheStatsDimension("dim2", "B")); - StatsHolder.Key matchingKey = new StatsHolder.Key(List.of("A", "B", "C")); - StatsHolder.Key nonMatchingKey = new StatsHolder.Key(List.of("A", "Z", "C")); + StatsHolder.Key matchingKey = new StatsHolder.Key(List.of( + new CacheStatsDimension("dim1", "A"), + new CacheStatsDimension("dim2", "B"), + new CacheStatsDimension("dim3", "C") + )); + StatsHolder.Key nonMatchingKey = new StatsHolder.Key(List.of( + new CacheStatsDimension("dim1", "A"), + new CacheStatsDimension("dim2", "Z"), + new CacheStatsDimension("dim3", "C") + )); assertTrue(statsHolder.keyContainsAllDimensions(matchingKey, dims)); assertFalse(statsHolder.keyContainsAllDimensions(nonMatchingKey, dims));