Skip to content

Commit

Permalink
changed statsholder key to contain whole dimension
Browse files Browse the repository at this point in the history 
Signed-off-by: Peter Alfonsi <[email protected]>
  • Loading branch information
Peter Alfonsi committed Mar 29, 2024
1 parent 3a90973 commit e6dca8d
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 23 deletions.
8 changes: 4 additions & 4 deletions server/src/main/java/org/opensearch/common/cache/stats/MultiDimensionCacheStats.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public MultiDimensionCacheStats(Map<StatsHolder.Key, CounterSnapshot> 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
);
}
Expand All @@ -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)
);
}
Expand Down Expand Up @@ -158,9 +158,9 @@ DimensionNode aggregateByLevels(List<String> levels) {
DimensionNode root = new DimensionNode(null);
for (Map.Entry<StatsHolder.Key, CounterSnapshot> entry : snapshot.entrySet()) {
List<String> levelValues = new ArrayList<>(); // This key's relevant dimension values, which match the levels
List<String> keyDimensionValues = entry.getKey().dimensionValues;
List<CacheStatsDimension> 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());
Expand Down
28 changes: 16 additions & 12 deletions server/src/main/java/org/opensearch/common/cache/stats/StatsHolder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,17 +137,17 @@ private CacheStatsCounter internalGetStats(List<CacheStatsDimension> dimensions)
* Get a valid key from an unordered list of dimensions.
*/
private Key getKey(List<CacheStatsDimension> 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<String> getOrderedDimensionValues(List<CacheStatsDimension> dimensions, List<String> dimensionNames) {
List<String> result = new ArrayList<>();
public static List<CacheStatsDimension> getOrderedDimensions(List<CacheStatsDimension> dimensions, List<String> dimensionNames) {
List<CacheStatsDimension> result = new ArrayList<>();
for (String dimensionName : dimensionNames) {
for (CacheStatsDimension dim : dimensions) {
if (dim.dimensionName.equals(dimensionName)) {
result.add(dim.dimensionValue);
result.add(dim);
}
}
}
Expand Down Expand Up @@ -183,13 +183,17 @@ public void removeDimensions(List<CacheStatsDimension> dims) {
}
}

/**
* Check if the Key contains all the dimensions in dims, matching both dimension name and value.
*/
boolean keyContainsAllDimensions(Key key, List<CacheStatsDimension> 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;
}
}
Expand All @@ -200,14 +204,14 @@ boolean keyContainsAllDimensions(Key key, List<CacheStatsDimension> dims) {
* Unmodifiable wrapper over a list of dimension values, ordered according to dimensionNames. Pkg-private for testing.
*/
public static class Key {
final List<String> dimensionValues; // The dimensions must be ordered
final List<CacheStatsDimension> dimensions; // The dimensions must be ordered

public Key(List<String> dimensionValues) {
this.dimensionValues = Collections.unmodifiableList(dimensionValues);
public Key(List<CacheStatsDimension> dimensions) {
this.dimensions = Collections.unmodifiableList(dimensions);
}

public List<String> getDimensionValues() {
return dimensionValues;
public List<CacheStatsDimension> getDimensions() {
return dimensions;
}

@Override
Expand All @@ -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();
}
}
}
8 changes: 6 additions & 2 deletions server/src/test/java/org/opensearch/common/cache/stats/MultiDimensionCacheStatsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<CacheStatsDimension> 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);
Expand Down Expand Up @@ -134,10 +134,14 @@ public void testAggregateBySomeDimensions() throws Exception {
for (Map.Entry<List<String>, MultiDimensionCacheStats.DimensionNode> aggEntry : aggregatedLeafNodes.entrySet()) {
CacheStatsCounter expectedCounter = new CacheStatsCounter();
for (List<CacheStatsDimension> expectedDims : expected.keySet()) {
List<String> orderedDimValues = StatsHolder.getOrderedDimensionValues(
List<CacheStatsDimension> orderedDims = StatsHolder.getOrderedDimensions(
new ArrayList<>(expectedDims),
dimensionNames
);
List<String> orderedDimValues = new ArrayList<>();
for (CacheStatsDimension dim : orderedDims) {
orderedDimValues.add(dim.dimensionValue);
}
if (orderedDimValues.containsAll(aggEntry.getKey())) {
expectedCounter.add(expected.get(expectedDims));
}
Expand Down
26 changes: 21 additions & 5 deletions server/src/test/java/org/opensearch/common/cache/stats/StatsHolderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,18 @@ public class StatsHolderTests extends OpenSearchTestCase {
// in MultiDimensionCacheStatsTests.java.

public void testKeyEquality() throws Exception {
List<String> dims1 = List.of("1", "2", "3");
List<CacheStatsDimension> dims1 = List.of(
new CacheStatsDimension("A", "1"),
new CacheStatsDimension("B", "2"),
new CacheStatsDimension("C", "3")
);
StatsHolder.Key key1 = new StatsHolder.Key(dims1);

List<String> dims2 = List.of("1", "2", "3");
List<CacheStatsDimension> 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);
Expand All @@ -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);
}
Expand All @@ -68,8 +76,16 @@ public void testKeyContainsAllDimensions() throws Exception {

List<CacheStatsDimension> 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));
Expand Down

0 comments on commit e6dca8d

Please sign in to comment.