Skip to content

Commit

Permalink
Cleaned up logic for cache stats ITs
Browse files Browse the repository at this point in the history 
Signed-off-by: Peter Alfonsi <[email protected]>
  • Loading branch information
Peter Alfonsi committed May 13, 2024
1 parent a31a5a3 commit d47ccb6
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 79 deletions.
122 changes: 46 additions & 76 deletions ...c/internalClusterTest/java/org/opensearch/indices/CacheStatsAPIIndicesRequestCacheIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import org.opensearch.common.cache.CacheType;
import org.opensearch.common.cache.service.NodeCacheStats;
import org.opensearch.common.cache.stats.ImmutableCacheStats;
import org.opensearch.common.cache.stats.ImmutableCacheStatsHolderTests;
import org.opensearch.common.cache.stats.ImmutableCacheStatsHolder;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.common.xcontent.XContentFactory;
Expand Down Expand Up @@ -74,77 +74,52 @@ public void testCacheStatsAPIWIthOnHeapCache() throws Exception {
searchIndex(client, index2Name, "");

// First, aggregate by indices only
Map<String, Object> xContentMap = getNodeCacheStatsXContentMap(client, List.of(IndicesRequestCache.INDEX_DIMENSION_NAME));
ImmutableCacheStatsHolder indicesStats = getNodeCacheStatsResult(client, List.of(IndicesRequestCache.INDEX_DIMENSION_NAME));

List<String> index1Keys = List.of(CacheType.INDICES_REQUEST_CACHE.getValue(), IndicesRequestCache.INDEX_DIMENSION_NAME, index1Name);
List<String> index1Dimensions = List.of(index1Name);
// Since we searched twice, we expect to see 1 hit, 1 miss and 1 entry for index 1
ImmutableCacheStats expectedStats = new ImmutableCacheStats(1, 1, 0, 0, 1);
checkCacheStatsAPIResponse(xContentMap, index1Keys, expectedStats, false, true);
checkCacheStatsAPIResponse(indicesStats, index1Dimensions, expectedStats, false, true);
// Get the request size for one request, so we can reuse it for next index
int requestSize = (int) ((Map<String, Object>) ImmutableCacheStatsHolderTests.getValueFromNestedXContentMap(
xContentMap,
index1Keys
)).get(ImmutableCacheStats.Fields.SIZE_IN_BYTES);
long requestSize = indicesStats.getStatsForDimensionValues(List.of(index1Name)).getSizeInBytes();
assertTrue(requestSize > 0);

List<String> index2Keys = List.of(CacheType.INDICES_REQUEST_CACHE.getValue(), IndicesRequestCache.INDEX_DIMENSION_NAME, index2Name);
List<String> index2Dimensions = List.of(index2Name);
// We searched once in index 2, we expect 1 miss + 1 entry
expectedStats = new ImmutableCacheStats(0, 1, 0, requestSize, 1);
checkCacheStatsAPIResponse(xContentMap, index2Keys, expectedStats, true, true);
checkCacheStatsAPIResponse(indicesStats, index2Dimensions, expectedStats, true, true);

// The total stats for the node should be 1 hit, 2 misses, and 2 entries
expectedStats = new ImmutableCacheStats(1, 2, 0, 2 * requestSize, 2);
List<String> totalStatsKeys = List.of(CacheType.INDICES_REQUEST_CACHE.getValue());
checkCacheStatsAPIResponse(xContentMap, totalStatsKeys, expectedStats, true, true);
List<String> totalStatsKeys = List.of();
checkCacheStatsAPIResponse(indicesStats, totalStatsKeys, expectedStats, true, true);

// Aggregate by shards only
xContentMap = getNodeCacheStatsXContentMap(client, List.of(IndicesRequestCache.SHARD_ID_DIMENSION_NAME));
ImmutableCacheStatsHolder shardsStats = getNodeCacheStatsResult(client, List.of(IndicesRequestCache.SHARD_ID_DIMENSION_NAME));

List<String> index1Shard0Keys = List.of(
CacheType.INDICES_REQUEST_CACHE.getValue(),
IndicesRequestCache.SHARD_ID_DIMENSION_NAME,
"[" + index1Name + "][0]"
);
List<String> index1Shard0Dimensions = List.of("[" + index1Name + "][0]");

expectedStats = new ImmutableCacheStats(1, 1, 0, requestSize, 1);
checkCacheStatsAPIResponse(xContentMap, index1Shard0Keys, expectedStats, true, true);
checkCacheStatsAPIResponse(shardsStats, index1Shard0Dimensions, expectedStats, true, true);

List<String> index2Shard0Keys = List.of(
CacheType.INDICES_REQUEST_CACHE.getValue(),
IndicesRequestCache.SHARD_ID_DIMENSION_NAME,
"[" + index2Name + "][0]"
);
List<String> index2Shard0Dimensions = List.of("[" + index2Name + "][0]");
expectedStats = new ImmutableCacheStats(0, 1, 0, requestSize, 1);
checkCacheStatsAPIResponse(xContentMap, index2Shard0Keys, expectedStats, true, true);
checkCacheStatsAPIResponse(shardsStats, index2Shard0Dimensions, expectedStats, true, true);

// Aggregate by indices and shards
xContentMap = getNodeCacheStatsXContentMap(
ImmutableCacheStatsHolder indicesAndShardsStats = getNodeCacheStatsResult(
client,
List.of(IndicesRequestCache.INDEX_DIMENSION_NAME, IndicesRequestCache.SHARD_ID_DIMENSION_NAME)
);

index1Keys = List.of(
CacheType.INDICES_REQUEST_CACHE.getValue(),
IndicesRequestCache.INDEX_DIMENSION_NAME,
index1Name,
IndicesRequestCache.SHARD_ID_DIMENSION_NAME,
"[" + index1Name + "][0]"
);
index1Dimensions = List.of(index1Name, "[" + index1Name + "][0]");

expectedStats = new ImmutableCacheStats(1, 1, 0, requestSize, 1);
checkCacheStatsAPIResponse(xContentMap, index1Keys, expectedStats, true, true);

index2Keys = List.of(
CacheType.INDICES_REQUEST_CACHE.getValue(),
IndicesRequestCache.INDEX_DIMENSION_NAME,
index2Name,
IndicesRequestCache.SHARD_ID_DIMENSION_NAME,
"[" + index2Name + "][0]"
);
checkCacheStatsAPIResponse(indicesAndShardsStats, index1Dimensions, expectedStats, true, true);

index2Dimensions = List.of(index2Name, "[" + index2Name + "][0]");
expectedStats = new ImmutableCacheStats(0, 1, 0, requestSize, 1);
checkCacheStatsAPIResponse(xContentMap, index2Keys, expectedStats, true, true);

checkCacheStatsAPIResponse(indicesAndShardsStats, index2Dimensions, expectedStats, true, true);
}

public void testStatsMatchOldApi() throws Exception {
Expand Down Expand Up @@ -172,8 +147,7 @@ public void testStatsMatchOldApi() throws Exception {
.getRequestCache();
assertNotEquals(0, oldApiStats.getMemorySizeInBytes());

List<String> xContentMapKeys = List.of(CacheType.INDICES_REQUEST_CACHE.getValue());
Map<String, Object> xContentMap = getNodeCacheStatsXContentMap(client, List.of());
ImmutableCacheStatsHolder statsHolder = getNodeCacheStatsResult(client, List.of());
ImmutableCacheStats expected = new ImmutableCacheStats(
oldApiStats.getHitCount(),
oldApiStats.getMissCount(),
Expand All @@ -182,20 +156,21 @@ public void testStatsMatchOldApi() throws Exception {
0
);
// Don't check entries, as the old API doesn't track this
checkCacheStatsAPIResponse(xContentMap, xContentMapKeys, expected, true, false);
checkCacheStatsAPIResponse(statsHolder, List.of(), expected, true, false);
}

public void testNullLevels() throws Exception {
// Test the XContent in the response behaves correctly when we pass null levels.
String index = "index";
Client client = client();
startIndex(client, index);
int numKeys = Randomness.get().nextInt(100) + 1;
for (int i = 0; i < numKeys; i++) {
searchIndex(client, index, String.valueOf(i));
}
Map<String, Object> xContentMap = getNodeCacheStatsXContentMap(client, null);
Map<String, Object> xContentMap = getStatsXContent(getNodeCacheStatsResult(client, null));
// Null levels should result in only the total cache stats being returned -> 6 fields inside the response.
assertEquals(6, ((Map<String, Object>) xContentMap.get("request_cache")).size());
assertEquals(6, xContentMap.size()); // ((Map<String, Object>) xContentMap.get("request_cache")).size());
}

public void testCacheClear() throws Exception {
Expand All @@ -217,10 +192,13 @@ public void testCacheClear() throws Exception {
}

ImmutableCacheStats expectedTotal = new ImmutableCacheStats(expectedHits, expectedMisses, 0, 0, expectedMisses);
Map<String, Object> xContentMap = getNodeCacheStatsXContentMap(client, List.of());
ImmutableCacheStatsHolder statsHolder = getNodeCacheStatsResult(client, List.of());
// Don't check the memory size, just assert it's nonzero
Map<String, Object> totalStatsResponse = checkCacheStatsAPIResponse(xContentMap, List.of(CacheType.INDICES_REQUEST_CACHE.getValue()), expectedTotal, false, true);
int originalMemorySize = (int) totalStatsResponse.get(ImmutableCacheStats.Fields.SIZE_IN_BYTES);
checkCacheStatsAPIResponse(statsHolder, List.of(), expectedTotal, false, true);
// Map<String, Object> totalStatsResponse = checkCacheStatsAPIResponse(xContentMap,
// List.of(CacheType.INDICES_REQUEST_CACHE.getValue()), expectedTotal, false, true);
long originalMemorySize = statsHolder.getTotalSizeInBytes(); // (int)
// totalStatsResponse.get(ImmutableCacheStats.Fields.SIZE_IN_BYTES);
assertNotEquals(0, originalMemorySize);

// Clear cache
Expand All @@ -229,9 +207,8 @@ public void testCacheClear() throws Exception {

// Now size and items should be 0
expectedTotal = new ImmutableCacheStats(expectedHits, expectedMisses, 0, 0, 0);
xContentMap = getNodeCacheStatsXContentMap(client, List.of());
checkCacheStatsAPIResponse(xContentMap, List.of(CacheType.INDICES_REQUEST_CACHE.getValue()), expectedTotal, true, true);

statsHolder = getNodeCacheStatsResult(client, List.of());
checkCacheStatsAPIResponse(statsHolder, List.of(), expectedTotal, true, true);
}

private void startIndex(Client client, String indexName) throws InterruptedException {
Expand Down Expand Up @@ -262,8 +239,7 @@ private SearchResponse searchIndex(Client client, String index, String searchSuf
return resp;
}

private static Map<String, Object> getNodeCacheStatsXContentMap(Client client, List<String> aggregationLevels) throws IOException {

private static ImmutableCacheStatsHolder getNodeCacheStatsResult(Client client, List<String> aggregationLevels) throws IOException {
CommonStatsFlags statsFlags = new CommonStatsFlags();
statsFlags.includeAllCacheTypes();
String[] flagsLevels;
Expand All @@ -283,45 +259,39 @@ private static Map<String, Object> getNodeCacheStatsXContentMap(Client client, L
// Can always get the first data node as there's only one in this test suite
assertEquals(1, nodeStatsResponse.getNodes().size());
NodeCacheStats ncs = nodeStatsResponse.getNodes().get(0).getNodeCacheStats();
return ncs.getStatsByCache(CacheType.INDICES_REQUEST_CACHE);
}

private static Map<String, Object> getStatsXContent(ImmutableCacheStatsHolder statsHolder) throws IOException {
XContentBuilder builder = XContentFactory.jsonBuilder();
Map<String, String> paramMap = new HashMap<>();
if (aggregationLevels != null && !aggregationLevels.isEmpty()) {
paramMap.put("level", String.join(",", aggregationLevels));
}
ToXContent.Params params = new ToXContent.MapParams(paramMap);

builder.startObject();
ncs.toXContent(builder, params);
statsHolder.toXContent(builder, params);
builder.endObject();

String resultString = builder.toString();
return XContentHelper.convertToMap(MediaTypeRegistry.JSON.xContent(), resultString, true);
}

private static Map<String, Object> checkCacheStatsAPIResponse(
Map<String, Object> xContentMap,
List<String> xContentMapKeys,
private static void checkCacheStatsAPIResponse(
ImmutableCacheStatsHolder statsHolder,
List<String> dimensionValues,
ImmutableCacheStats expectedStats,
boolean checkMemorySize,
boolean checkEntries
) {
// Assumes the keys point to a level whose keys are the field values ("size_in_bytes", "evictions", etc) and whose values store
// those stats
Map<String, Object> aggregatedStatsResponse = (Map<String, Object>) ImmutableCacheStatsHolderTests.getValueFromNestedXContentMap(
xContentMap,
xContentMapKeys
);
ImmutableCacheStats aggregatedStatsResponse = statsHolder.getStatsForDimensionValues(dimensionValues);
assertNotNull(aggregatedStatsResponse);
assertEquals(expectedStats.getHits(), (int) aggregatedStatsResponse.get(ImmutableCacheStats.Fields.HIT_COUNT));
assertEquals(expectedStats.getMisses(), (int) aggregatedStatsResponse.get(ImmutableCacheStats.Fields.MISS_COUNT));
assertEquals(expectedStats.getEvictions(), (int) aggregatedStatsResponse.get(ImmutableCacheStats.Fields.EVICTIONS));
assertEquals(expectedStats.getHits(), (int) aggregatedStatsResponse.getHits());
assertEquals(expectedStats.getMisses(), (int) aggregatedStatsResponse.getMisses());
assertEquals(expectedStats.getEvictions(), (int) aggregatedStatsResponse.getEvictions());
if (checkMemorySize) {
assertEquals(expectedStats.getSizeInBytes(), (int) aggregatedStatsResponse.get(ImmutableCacheStats.Fields.SIZE_IN_BYTES));
assertEquals(expectedStats.getSizeInBytes(), (int) aggregatedStatsResponse.getSizeInBytes());
}
if (checkEntries) {
assertEquals(expectedStats.getItems(), (int) aggregatedStatsResponse.get(ImmutableCacheStats.Fields.ITEM_COUNT));
assertEquals(expectedStats.getItems(), (int) aggregatedStatsResponse.getItems());
}
return aggregatedStatsResponse;
}
}
6 changes: 6 additions & 0 deletions server/src/main/java/org/opensearch/common/cache/service/NodeCacheStats.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,10 @@ public boolean equals(Object o) {
public int hashCode() {
return Objects.hash(statsByCache, flags);
}

// Get the immutable cache stats for a given cache, used to avoid having to process XContent in tests.
// Safe to expose publicly as the ImmutableCacheStatsHolder can't be modified after its creation.
public ImmutableCacheStatsHolder getStatsByCache(CacheType cacheType) {
return statsByCache.get(cacheType);
}
}
3 changes: 0 additions & 3 deletions server/src/main/java/org/opensearch/indices/IndicesRequestCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -735,9 +735,6 @@ private synchronized void cleanCache(double stalenessThreshold) {
} else {
cleanupKeysFromOutdatedReaders.add(cleanupKey);
}



}

if (cleanupKeysFromOutdatedReaders.isEmpty() && cleanupKeysFromFullClean.isEmpty() && cleanupKeysFromClosedShards.isEmpty()) {
Expand Down

0 comments on commit d47ccb6

Please sign in to comment.