Skip to content

Commit b5bded2

Browse files
author
Peter Alfonsi
committed
changed statsholder key to contain whole dimension
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
1 parent fb3baaa commit b5bded2

File tree

4 files changed

+47
-23
lines changed

4 files changed

+47
-23
lines changed

‎server/src/main/java/org/opensearch/common/cache/stats/MultiDimensionCacheStats.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public MultiDimensionCacheStats(Map<StatsHolder.Key, CounterSnapshot> snapshot,
3737
public MultiDimensionCacheStats(StreamInput in) throws IOException {
3838
this.dimensionNames = List.of(in.readStringArray());
3939
this.snapshot = in.readMap(
40-
i -> new StatsHolder.Key(List.of(i.readArray(StreamInput::readString, String[]::new))),
40+
i -> new StatsHolder.Key(List.of(i.readArray(CacheStatsDimension::new, CacheStatsDimension[]::new))),
4141
CounterSnapshot::new
4242
);
4343
}
@@ -47,7 +47,7 @@ public void writeTo(StreamOutput out) throws IOException {
4747
out.writeStringArray(dimensionNames.toArray(new String[0]));
4848
out.writeMap(
4949
snapshot,
50-
(o, key) -> o.writeArray((o1, dimValue) -> o1.writeString((String) dimValue), key.dimensionValues.toArray()),
50+
(o, key) -> o.writeArray((o1, dim) -> ((CacheStatsDimension) dim).writeTo(o1), key.dimensions.toArray()),
5151
(o, snapshot) -> snapshot.writeTo(o)
5252
);
5353
}
@@ -140,9 +140,9 @@ DimensionNode aggregateByLevels(List<String> levels) {
140140
DimensionNode root = new DimensionNode(null);
141141
for (Map.Entry<StatsHolder.Key, CounterSnapshot> entry : snapshot.entrySet()) {
142142
List<String> levelValues = new ArrayList<>(); // This key's relevant dimension values, which match the levels
143-
List<String> keyDimensionValues = entry.getKey().dimensionValues;
143+
List<CacheStatsDimension> keyDimensions = entry.getKey().dimensions;
144144
for (int levelPosition : levelPositions) {
145-
levelValues.add(keyDimensionValues.get(levelPosition));
145+
levelValues.add(keyDimensions.get(levelPosition).dimensionValue);
146146
}
147147
DimensionNode leafNode = root.getNode(levelValues);
148148
leafNode.addSnapshot(entry.getValue());

‎server/src/main/java/org/opensearch/common/cache/stats/StatsHolder.java

+16-12
Original file line numberDiff line numberDiff line change
@@ -137,17 +137,17 @@ private CacheStatsCounter internalGetStats(List<CacheStatsDimension> dimensions)
137137
* Get a valid key from an unordered list of dimensions.
138138
*/
139139
private Key getKey(List<CacheStatsDimension> dims) {
140-
return new Key(getOrderedDimensionValues(dims, dimensionNames));
140+
return new Key(getOrderedDimensions(dims, dimensionNames));
141141
}
142142

143143
// Get a list of dimension values, ordered according to dimensionNames, from the possibly differently-ordered dimensions passed in.
144144
// Public and static for testing purposes.
145-
public static List<String> getOrderedDimensionValues(List<CacheStatsDimension> dimensions, List<String> dimensionNames) {
146-
List<String> result = new ArrayList<>();
145+
public static List<CacheStatsDimension> getOrderedDimensions(List<CacheStatsDimension> dimensions, List<String> dimensionNames) {
146+
List<CacheStatsDimension> result = new ArrayList<>();
147147
for (String dimensionName : dimensionNames) {
148148
for (CacheStatsDimension dim : dimensions) {
149149
if (dim.dimensionName.equals(dimensionName)) {
150-
result.add(dim.dimensionValue);
150+
result.add(dim);
151151
}
152152
}
153153
}
@@ -183,13 +183,17 @@ public void removeDimensions(List<CacheStatsDimension> dims) {
183183
}
184184
}
185185

186+
/**
187+
* Check if the Key contains all the dimensions in dims, matching both dimension name and value.
188+
*/
186189
boolean keyContainsAllDimensions(Key key, List<CacheStatsDimension> dims) {
187190
for (CacheStatsDimension dim : dims) {
188191
int dimensionPosition = dimensionNames.indexOf(dim.dimensionName);
189192
if (dimensionPosition == -1) {
190193
throw new IllegalArgumentException("Unrecognized dimension: " + dim.dimensionName + " = " + dim.dimensionValue);
191194
}
192-
if (!key.dimensionValues.get(dimensionPosition).equals(dim.dimensionValue)) {
195+
String keyDimensionValue = key.dimensions.get(dimensionPosition).dimensionValue;
196+
if (!keyDimensionValue.equals(dim.dimensionValue)) {
193197
return false;
194198
}
195199
}
@@ -200,14 +204,14 @@ boolean keyContainsAllDimensions(Key key, List<CacheStatsDimension> dims) {
200204
* Unmodifiable wrapper over a list of dimension values, ordered according to dimensionNames. Pkg-private for testing.
201205
*/
202206
public static class Key {
203-
final List<String> dimensionValues; // The dimensions must be ordered
207+
final List<CacheStatsDimension> dimensions; // The dimensions must be ordered
204208

205-
public Key(List<String> dimensionValues) {
206-
this.dimensionValues = Collections.unmodifiableList(dimensionValues);
209+
public Key(List<CacheStatsDimension> dimensions) {
210+
this.dimensions = Collections.unmodifiableList(dimensions);
207211
}
208212

209-
public List<String> getDimensionValues() {
210-
return dimensionValues;
213+
public List<CacheStatsDimension> getDimensions() {
214+
return dimensions;
211215
}
212216

213217
@Override
@@ -222,12 +226,12 @@ public boolean equals(Object o) {
222226
return false;
223227
}
224228
Key other = (Key) o;
225-
return this.dimensionValues.equals(other.dimensionValues);
229+
return this.dimensions.equals(other.dimensions);
226230
}
227231

228232
@Override
229233
public int hashCode() {
230-
return this.dimensionValues.hashCode();
234+
return this.dimensions.hashCode();
231235
}
232236
}
233237
}

‎server/src/test/java/org/opensearch/common/cache/stats/MultiDimensionCacheStatsTests.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public void testAddAndGet() throws Exception {
5151
// test the value in the map is as expected for each distinct combination of values
5252
for (List<CacheStatsDimension> dims : expected.keySet()) {
5353
CacheStatsCounter expectedCounter = expected.get(dims);
54-
StatsHolder.Key key = new StatsHolder.Key(StatsHolder.getOrderedDimensionValues(dims, dimensionNames));
54+
StatsHolder.Key key = new StatsHolder.Key(StatsHolder.getOrderedDimensions(dims, dimensionNames));
5555
CounterSnapshot actual = stats.snapshot.get(key);
5656

5757
assertEquals(expectedCounter.snapshot(), actual);
@@ -124,10 +124,14 @@ public void testAggregateBySomeDimensions() throws Exception {
124124
for (Map.Entry<List<String>, MultiDimensionCacheStats.DimensionNode> aggEntry : aggregatedLeafNodes.entrySet()) {
125125
CacheStatsCounter expectedCounter = new CacheStatsCounter();
126126
for (List<CacheStatsDimension> expectedDims : expected.keySet()) {
127-
List<String> orderedDimValues = StatsHolder.getOrderedDimensionValues(
127+
List<CacheStatsDimension> orderedDims = StatsHolder.getOrderedDimensions(
128128
new ArrayList<>(expectedDims),
129129
dimensionNames
130130
);
131+
List<String> orderedDimValues = new ArrayList<>();
132+
for (CacheStatsDimension dim : orderedDims) {
133+
orderedDimValues.add(dim.dimensionValue);
134+
}
131135
if (orderedDimValues.containsAll(aggEntry.getKey())) {
132136
expectedCounter.add(expected.get(expectedDims));
133137
}

‎server/src/test/java/org/opensearch/common/cache/stats/StatsHolderTests.java

+21-5
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,18 @@ public class StatsHolderTests extends OpenSearchTestCase {
2727
// in MultiDimensionCacheStatsTests.java.
2828

2929
public void testKeyEquality() throws Exception {
30-
List<String> dims1 = List.of("1", "2", "3");
30+
List<CacheStatsDimension> dims1 = List.of(
31+
new CacheStatsDimension("A", "1"),
32+
new CacheStatsDimension("B", "2"),
33+
new CacheStatsDimension("C", "3")
34+
);
3135
StatsHolder.Key key1 = new StatsHolder.Key(dims1);
3236

33-
List<String> dims2 = List.of("1", "2", "3");
37+
List<CacheStatsDimension> dims2 = List.of(
38+
new CacheStatsDimension("A", "1"),
39+
new CacheStatsDimension("B", "2"),
40+
new CacheStatsDimension("C", "3")
41+
);
3442
StatsHolder.Key key2 = new StatsHolder.Key(dims2);
3543

3644
assertEquals(key1, key2);
@@ -50,7 +58,7 @@ public void testReset() throws Exception {
5058
originalCounter.sizeInBytes = new CounterMetric();
5159
originalCounter.entries = new CounterMetric();
5260

53-
StatsHolder.Key key = new StatsHolder.Key(StatsHolder.getOrderedDimensionValues(dims, dimensionNames));
61+
StatsHolder.Key key = new StatsHolder.Key(StatsHolder.getOrderedDimensions(dims, dimensionNames));
5462
CacheStatsCounter actual = statsHolder.getStatsMap().get(key);
5563
assertEquals(originalCounter, actual);
5664
}
@@ -69,8 +77,16 @@ public void testKeyContainsAllDimensions() throws Exception {
6977

7078
List<CacheStatsDimension> dims = List.of(new CacheStatsDimension("dim1", "A"), new CacheStatsDimension("dim2", "B"));
7179

72-
StatsHolder.Key matchingKey = new StatsHolder.Key(List.of("A", "B", "C"));
73-
StatsHolder.Key nonMatchingKey = new StatsHolder.Key(List.of("A", "Z", "C"));
80+
StatsHolder.Key matchingKey = new StatsHolder.Key(List.of(
81+
new CacheStatsDimension("dim1", "A"),
82+
new CacheStatsDimension("dim2", "B"),
83+
new CacheStatsDimension("dim3", "C")
84+
));
85+
StatsHolder.Key nonMatchingKey = new StatsHolder.Key(List.of(
86+
new CacheStatsDimension("dim1", "A"),
87+
new CacheStatsDimension("dim2", "Z"),
88+
new CacheStatsDimension("dim3", "C")
89+
));
7490

7591
assertTrue(statsHolder.keyContainsAllDimensions(matchingKey, dims));
7692
assertFalse(statsHolder.keyContainsAllDimensions(nonMatchingKey, dims));

0 commit comments

Comments
 (0)