Skip to content

Commit

Permalink
Addressed Michael's comments
Browse files Browse the repository at this point in the history
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
  • Loading branch information
Peter Alfonsi committed Apr 25, 2024
1 parent b5b027a commit 40796c5
Show file tree
Hide file tree
Showing 16 changed files with 72 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -509,15 +509,15 @@ private long getNewValuePairSize(CacheEvent<? extends ICacheKey<K>, ? extends By
public void onEvent(CacheEvent<? extends ICacheKey<K>, ? extends ByteArrayWrapper> event) {
switch (event.getType()) {
case CREATED:
cacheStatsHolder.incrementEntries(event.getKey().dimensions);
cacheStatsHolder.incrementItems(event.getKey().dimensions);
cacheStatsHolder.incrementSizeInBytes(event.getKey().dimensions, getNewValuePairSize(event));
assert event.getOldValue() == null;
break;
case EVICTED:
this.removalListener.onRemoval(
new RemovalNotification<>(event.getKey(), deserializeValue(event.getOldValue()), RemovalReason.EVICTED)
);
cacheStatsHolder.decrementEntries(event.getKey().dimensions);
cacheStatsHolder.decrementItems(event.getKey().dimensions);
cacheStatsHolder.decrementSizeInBytes(event.getKey().dimensions, getOldValuePairSize(event));
cacheStatsHolder.incrementEvictions(event.getKey().dimensions);
assert event.getNewValue() == null;
Expand All @@ -526,15 +526,15 @@ public void onEvent(CacheEvent<? extends ICacheKey<K>, ? extends ByteArrayWrappe
this.removalListener.onRemoval(
new RemovalNotification<>(event.getKey(), deserializeValue(event.getOldValue()), RemovalReason.EXPLICIT)
);
cacheStatsHolder.decrementEntries(event.getKey().dimensions);
cacheStatsHolder.decrementItems(event.getKey().dimensions);
cacheStatsHolder.decrementSizeInBytes(event.getKey().dimensions, getOldValuePairSize(event));
assert event.getNewValue() == null;
break;
case EXPIRED:
this.removalListener.onRemoval(
new RemovalNotification<>(event.getKey(), deserializeValue(event.getOldValue()), RemovalReason.INVALIDATED)
);
cacheStatsHolder.decrementEntries(event.getKey().dimensions);
cacheStatsHolder.decrementItems(event.getKey().dimensions);
cacheStatsHolder.decrementSizeInBytes(event.getKey().dimensions, getOldValuePairSize(event));
assert event.getNewValue() == null;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public void testBasicGetAndPut() throws IOException {
String value = ehcacheTest.get(getICacheKey(entry.getKey()));
assertEquals(entry.getValue(), value);
}
assertEquals(randomKeys, ehcacheTest.stats().getTotalEntries());
assertEquals(randomKeys, ehcacheTest.stats().getTotalItems());
assertEquals(randomKeys, ehcacheTest.stats().getTotalHits());
assertEquals(expectedSize, ehcacheTest.stats().getTotalSizeInBytes());
assertEquals(randomKeys, ehcacheTest.count());
Expand Down Expand Up @@ -217,7 +217,7 @@ public void testConcurrentPut() throws Exception {
assertEquals(entry.getValue(), value);
}
assertEquals(randomKeys, ehcacheTest.count());
assertEquals(randomKeys, ehcacheTest.stats().getTotalEntries());
assertEquals(randomKeys, ehcacheTest.stats().getTotalItems());
ehcacheTest.close();
}
}
Expand Down Expand Up @@ -416,7 +416,7 @@ public String load(ICacheKey<String> key) {
assertEquals(1, numberOfTimesValueLoaded);
assertEquals(0, ((EhcacheDiskCache) ehcacheTest).getCompletableFutureMap().size());
assertEquals(1, ehcacheTest.stats().getTotalMisses());
assertEquals(1, ehcacheTest.stats().getTotalEntries());
assertEquals(1, ehcacheTest.stats().getTotalItems());
assertEquals(numberOfRequest - 1, ehcacheTest.stats().getTotalHits());
assertEquals(1, ehcacheTest.count());
ehcacheTest.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ private static Map<String, Object> getNodeCacheStatsXContentMap(Client client, L
NodesStatsResponse nodeStatsResponse = client.admin()
.cluster()
.prepareNodesStats("data:true")
.addMetric(NodesStatsRequest.Metric.SEARCH_CACHE_STATS.metricName())
.addMetric(NodesStatsRequest.Metric.CACHE_STATS.metricName())
.setIndices(statsFlags)
.get();
// Can always get the first data node as there's only one in this test suite
Expand Down Expand Up @@ -288,7 +288,7 @@ private static void checkCacheStatsAPIResponse(
assertEquals(expectedStats.getSizeInBytes(), (int) aggregatedStatsResponse.get(ImmutableCacheStats.Fields.SIZE_IN_BYTES));
}
if (checkEntries) {
assertEquals(expectedStats.getEntries(), (int) aggregatedStatsResponse.get(ImmutableCacheStats.Fields.ENTRIES));
assertEquals(expectedStats.getItems(), (int) aggregatedStatsResponse.get(ImmutableCacheStats.Fields.ITEM_COUNT));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public enum Metric {
SEGMENT_REPLICATION_BACKPRESSURE("segment_replication_backpressure"),
REPOSITORIES("repositories"),
ADMISSION_CONTROL("admission_control"),
SEARCH_CACHE_STATS("caches");
CACHE_STATS("caches");

private String metricName;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ protected NodeStats nodeOperation(NodeStatsRequest nodeStatsRequest) {
NodesStatsRequest.Metric.SEGMENT_REPLICATION_BACKPRESSURE.containedIn(metrics),
NodesStatsRequest.Metric.REPOSITORIES.containedIn(metrics),
NodesStatsRequest.Metric.ADMISSION_CONTROL.containedIn(metrics),
NodesStatsRequest.Metric.SEARCH_CACHE_STATS.containedIn(metrics)
NodesStatsRequest.Metric.CACHE_STATS.containedIn(metrics)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.io.IOException;
import java.util.Collections;
import java.util.EnumSet;
import java.util.Set;

/**
* Common Stats Flags for OpenSearch
Expand Down Expand Up @@ -167,7 +168,7 @@ public Flag[] getFlags() {
return flags.toArray(new Flag[0]);
}

public EnumSet<CacheType> getIncludeCaches() {
public Set<CacheType> getIncludeCaches() {
return includeCaches;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public enum CacheType {
INDICES_REQUEST_CACHE("indices.requests.cache", "request_cache");

private final String settingPrefix;
private final String value; // The value displayed for this cache type in API responses
private final String value; // The value displayed for this cache type in stats API responses

private static final Map<String, CacheType> valuesMap;
static {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ public class CacheStats {
CounterMetric misses;
CounterMetric evictions;
CounterMetric sizeInBytes;
CounterMetric entries;
CounterMetric items;

public CacheStats(long hits, long misses, long evictions, long sizeInBytes, long entries) {
public CacheStats(long hits, long misses, long evictions, long sizeInBytes, long items) {
this.hits = new CounterMetric();
this.hits.inc(hits);
this.misses = new CounterMetric();
Expand All @@ -31,8 +31,8 @@ public CacheStats(long hits, long misses, long evictions, long sizeInBytes, long
this.evictions.inc(evictions);
this.sizeInBytes = new CounterMetric();
this.sizeInBytes.inc(sizeInBytes);
this.entries = new CounterMetric();
this.entries.inc(entries);
this.items = new CounterMetric();
this.items.inc(items);
}

public CacheStats() {
Expand All @@ -44,33 +44,33 @@ private void internalAdd(long otherHits, long otherMisses, long otherEvictions,
this.misses.inc(otherMisses);
this.evictions.inc(otherEvictions);
this.sizeInBytes.inc(otherSizeInBytes);
this.entries.inc(otherEntries);
this.items.inc(otherEntries);
}

public void add(CacheStats other) {
if (other == null) {
return;
}
internalAdd(other.getHits(), other.getMisses(), other.getEvictions(), other.getSizeInBytes(), other.getEntries());
internalAdd(other.getHits(), other.getMisses(), other.getEvictions(), other.getSizeInBytes(), other.getItems());
}

public void add(ImmutableCacheStats snapshot) {
if (snapshot == null) {
return;
}
internalAdd(snapshot.getHits(), snapshot.getMisses(), snapshot.getEvictions(), snapshot.getSizeInBytes(), snapshot.getEntries());
internalAdd(snapshot.getHits(), snapshot.getMisses(), snapshot.getEvictions(), snapshot.getSizeInBytes(), snapshot.getItems());
}

public void subtract(ImmutableCacheStats other) {
if (other == null) {
return;
}
internalAdd(-other.getHits(), -other.getMisses(), -other.getEvictions(), -other.getSizeInBytes(), -other.getEntries());
internalAdd(-other.getHits(), -other.getMisses(), -other.getEvictions(), -other.getSizeInBytes(), -other.getItems());
}

@Override
public int hashCode() {
return Objects.hash(hits.count(), misses.count(), evictions.count(), sizeInBytes.count(), entries.count());
return Objects.hash(hits.count(), misses.count(), evictions.count(), sizeInBytes.count(), items.count());
}

public void incrementHits() {
Expand All @@ -93,12 +93,12 @@ public void decrementSizeInBytes(long amount) {
sizeInBytes.dec(amount);
}

public void incrementEntries() {
entries.inc();
public void incrementItems() {
items.inc();
}

public void decrementEntries() {
entries.dec();
public void decrementItems() {
items.dec();
}

public long getHits() {
Expand All @@ -117,16 +117,16 @@ public long getSizeInBytes() {
return sizeInBytes.count();
}

public long getEntries() {
return entries.count();
public long getItems() {
return items.count();
}

public void resetSizeAndEntries() {
sizeInBytes = new CounterMetric();
entries = new CounterMetric();
items = new CounterMetric();
}

public ImmutableCacheStats immutableSnapshot() {
return new ImmutableCacheStats(hits.count(), misses.count(), evictions.count(), sizeInBytes.count(), entries.count());
return new ImmutableCacheStats(hits.count(), misses.count(), evictions.count(), sizeInBytes.count(), items.count());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ public void decrementSizeInBytes(List<String> dimensionValues, long amountBytes)
internalIncrement(dimensionValues, (node) -> node.decrementSizeInBytes(amountBytes), false);
}

public void incrementEntries(List<String> dimensionValues) {
internalIncrement(dimensionValues, Node::incrementEntries, true);
public void incrementItems(List<String> dimensionValues) {
internalIncrement(dimensionValues, Node::incrementItems, true);
}

public void decrementEntries(List<String> dimensionValues) {
internalIncrement(dimensionValues, Node::decrementEntries, false);
public void decrementItems(List<String> dimensionValues) {
internalIncrement(dimensionValues, Node::decrementItems, false);
}

/**
Expand All @@ -105,7 +105,7 @@ private void resetHelper(Node current) {

public long count() {
// Include this here so caches don't have to create an entire CacheStats object to run count().
return statsRoot.getEntries();
return statsRoot.getItems();
}

private void internalIncrement(List<String> dimensionValues, Consumer<Node> adder, boolean createNodesIfAbsent) {
Expand Down Expand Up @@ -255,16 +255,16 @@ void decrementSizeInBytes(long amountBytes) {
this.stats.decrementSizeInBytes(amountBytes);
}

void incrementEntries() {
this.stats.incrementEntries();
void incrementItems() {
this.stats.incrementItems();
}

void decrementEntries() {
this.stats.decrementEntries();
void decrementItems() {
this.stats.decrementItems();
}

long getEntries() {
return this.stats.getEntries();
long getItems() {
return this.stats.getItems();
}

ImmutableCacheStats getImmutableStats() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ public class ImmutableCacheStats implements Writeable, ToXContent {
private final long misses;
private final long evictions;
private final long sizeInBytes;
private final long entries;
private final long items;

public ImmutableCacheStats(long hits, long misses, long evictions, long sizeInBytes, long entries) {
public ImmutableCacheStats(long hits, long misses, long evictions, long sizeInBytes, long items) {
this.hits = hits;
this.misses = misses;
this.evictions = evictions;
this.sizeInBytes = sizeInBytes;
this.entries = entries;
this.items = items;
}

public ImmutableCacheStats(StreamInput in) throws IOException {
Expand All @@ -50,7 +50,7 @@ public static ImmutableCacheStats addSnapshots(ImmutableCacheStats s1, Immutable
s1.misses + s2.misses,
s1.evictions + s2.evictions,
s1.sizeInBytes + s2.sizeInBytes,
s1.entries + s2.entries
s1.items + s2.items
);
}

Expand All @@ -70,8 +70,8 @@ public long getSizeInBytes() {
return sizeInBytes;
}

public long getEntries() {
return entries;
public long getItems() {
return items;
}

@Override
Expand All @@ -80,7 +80,7 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeVLong(misses);
out.writeVLong(evictions);
out.writeVLong(sizeInBytes);
out.writeVLong(entries);
out.writeVLong(items);
}

@Override
Expand All @@ -96,12 +96,12 @@ public boolean equals(Object o) {
&& (misses == other.misses)
&& (evictions == other.evictions)
&& (sizeInBytes == other.sizeInBytes)
&& (entries == other.entries);
&& (items == other.items);
}

@Override
public int hashCode() {
return Objects.hash(hits, misses, evictions, sizeInBytes, entries);
return Objects.hash(hits, misses, evictions, sizeInBytes, items);
}

@Override
Expand All @@ -111,7 +111,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field(Fields.EVICTIONS, evictions);
builder.field(Fields.HIT_COUNT, hits);
builder.field(Fields.MISS_COUNT, misses);
builder.field(Fields.ENTRIES, entries);
builder.field(Fields.ITEM_COUNT, items);
return builder;
}

Expand All @@ -124,6 +124,6 @@ public static final class Fields {
public static final String EVICTIONS = "evictions";
public static final String HIT_COUNT = "hit_count";
public static final String MISS_COUNT = "miss_count";
public static final String ENTRIES = "entries";
public static final String ITEM_COUNT = "item_count";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ public long getTotalSizeInBytes() {
return getTotalStats().getSizeInBytes();
}

public long getTotalEntries() {
return getTotalStats().getEntries();
public long getTotalItems() {
return getTotalStats().getItems();
}

public ImmutableCacheStats getStatsForDimensionValues(List<String> dimensionValues) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public V get(ICacheKey<K> key) {
@Override
public void put(ICacheKey<K> key, V value) {
cache.put(key, value);
cacheStatsHolder.incrementEntries(key.dimensions);
cacheStatsHolder.incrementItems(key.dimensions);
cacheStatsHolder.incrementSizeInBytes(key.dimensions, weigher.applyAsLong(key, value));
}

Expand All @@ -92,7 +92,7 @@ public V computeIfAbsent(ICacheKey<K> key, LoadAwareCacheLoader<ICacheKey<K>, V>
cacheStatsHolder.incrementHits(key.dimensions);
} else {
cacheStatsHolder.incrementMisses(key.dimensions);
cacheStatsHolder.incrementEntries(key.dimensions);
cacheStatsHolder.incrementItems(key.dimensions);
cacheStatsHolder.incrementSizeInBytes(key.dimensions, cache.getWeigher().applyAsLong(key, value));
}
return value;
Expand Down Expand Up @@ -140,7 +140,7 @@ public ImmutableCacheStatsHolder stats(String[] levels) {
@Override
public void onRemoval(RemovalNotification<ICacheKey<K>, V> notification) {
removalListener.onRemoval(notification);
cacheStatsHolder.decrementEntries(notification.getKey().dimensions);
cacheStatsHolder.decrementItems(notification.getKey().dimensions);
cacheStatsHolder.decrementSizeInBytes(
notification.getKey().dimensions,
cache.getWeigher().applyAsLong(notification.getKey(), notification.getValue())
Expand Down
Loading

0 comments on commit 40796c5

Please sign in to comment.