Skip to content

Commit

Permalink
Fix flaky TSC stats tests
Browse files Browse the repository at this point in the history 
Signed-off-by: Peter Alfonsi <[email protected]>
  • Loading branch information
Peter Alfonsi committed Jun 13, 2024
1 parent 23458a4 commit 5cb11af
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 9 deletions.
36 changes: 27 additions & 9 deletions ...nternalClusterTest/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest;
import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse;
import org.opensearch.action.admin.indices.forcemerge.ForceMergeResponse;
import org.opensearch.action.admin.indices.stats.CommonStatsFlags;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.client.Client;
Expand All @@ -20,6 +21,7 @@
import org.opensearch.common.cache.stats.ImmutableCacheStatsHolder;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.cache.request.RequestCacheStats;
import org.opensearch.index.query.QueryBuilders;
import org.opensearch.indices.IndicesRequestCache;
Expand Down Expand Up @@ -102,8 +104,8 @@ public void testIndicesLevelAggregation() throws Exception {
);

for (ImmutableCacheStatsHolder statsHolder : List.of(allLevelsStatsHolder, indicesOnlyStatsHolder)) {
assertEquals(index1ExpectedStats, statsHolder.getStatsForDimensionValues(List.of(index1Name)));
assertEquals(index2ExpectedStats, statsHolder.getStatsForDimensionValues(List.of(index2Name)));
assertStatsEqual(index1ExpectedStats, statsHolder.getStatsForDimensionValues(List.of(index1Name)));
assertStatsEqual(index2ExpectedStats, statsHolder.getStatsForDimensionValues(List.of(index2Name)));
}
}

Expand Down Expand Up @@ -139,7 +141,7 @@ public void testIndicesAndTierLevelAggregation() throws Exception {
values.get("itemsOnHeapIndex1AfterTest")
)
);
assertEquals(
assertStatsEqual(
index1HeapExpectedStats,
allLevelsStatsHolder.getStatsForDimensionValues(List.of(index1Name, TIER_DIMENSION_VALUE_ON_HEAP))
);
Expand All @@ -153,7 +155,7 @@ public void testIndicesAndTierLevelAggregation() throws Exception {
values.get("itemsOnHeapIndex2AfterTest")
)
);
assertEquals(
assertStatsEqual(
index2HeapExpectedStats,
allLevelsStatsHolder.getStatsForDimensionValues(List.of(index2Name, TIER_DIMENSION_VALUE_ON_HEAP))
);
Expand All @@ -167,7 +169,7 @@ public void testIndicesAndTierLevelAggregation() throws Exception {
values.get("itemsOnDiskIndex1AfterTest")
)
);
assertEquals(
assertStatsEqual(
index1DiskExpectedStats,
allLevelsStatsHolder.getStatsForDimensionValues(List.of(index1Name, TIER_DIMENSION_VALUE_DISK))
);
Expand All @@ -181,7 +183,7 @@ public void testIndicesAndTierLevelAggregation() throws Exception {
values.get("itemsOnDiskIndex2AfterTest")
)
);
assertEquals(
assertStatsEqual(
index2DiskExpectedStats,
allLevelsStatsHolder.getStatsForDimensionValues(List.of(index2Name, TIER_DIMENSION_VALUE_DISK))
);
Expand Down Expand Up @@ -218,7 +220,7 @@ public void testTierLevelAggregation() throws Exception {
)
);
ImmutableCacheStats heapStats = tiersOnlyStatsHolder.getStatsForDimensionValues(List.of(TIER_DIMENSION_VALUE_ON_HEAP));
assertEquals(totalHeapExpectedStats, heapStats);
assertStatsEqual(totalHeapExpectedStats, heapStats);
ImmutableCacheStats totalDiskExpectedStats = returnNullIfAllZero(
new ImmutableCacheStats(
values.get("hitsOnDiskIndex1") + values.get("hitsOnDiskIndex2"),
Expand All @@ -229,7 +231,7 @@ public void testTierLevelAggregation() throws Exception {
)
);
ImmutableCacheStats diskStats = tiersOnlyStatsHolder.getStatsForDimensionValues(List.of(TIER_DIMENSION_VALUE_DISK));
assertEquals(totalDiskExpectedStats, diskStats);
assertStatsEqual(totalDiskExpectedStats, diskStats);
}

public void testInvalidLevelsAreIgnored() throws Exception {
Expand Down Expand Up @@ -322,7 +324,7 @@ public void testStatsMatchOldApi() throws Exception {
ImmutableCacheStats totalStats = getNodeCacheStatsResult(client, List.of()).getTotalStats();

// Check the new stats API values are as expected
assertEquals(
assertStatsEqual(
new ImmutableCacheStats(expectedHits, expectedMisses, 0, expectedEntries * singleSearchSize, expectedEntries),
totalStats
);
Expand Down Expand Up @@ -351,11 +353,15 @@ private void startIndex(Client client, String indexName) throws InterruptedExcep
.put(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), true)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
// Disable index refreshing to avoid cache being invalidated mid-test
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), TimeValue.timeValueMillis(-1))
.build()
)
.get()
);
indexRandom(true, client.prepareIndex(indexName).setSource("k", "hello"));
// Force merge the index to ensure there can be no background merges during the subsequent searches that would invalidate the cache
ForceMergeResponse forceMergeResponse = client.admin().indices().prepareForceMerge(indexName).setFlush(true).get();
ensureSearchable(indexName);
}

Expand Down Expand Up @@ -498,4 +504,16 @@ private static ImmutableCacheStatsHolder getNodeCacheStatsResult(Client client,
NodeCacheStats ncs = nodeStatsResponse.getNodes().get(0).getNodeCacheStats();
return ncs.getStatsByCache(CacheType.INDICES_REQUEST_CACHE);
}

// Check each stat separately for more transparency if there's a failure
private void assertStatsEqual(ImmutableCacheStats expected, ImmutableCacheStats actual) {
if (expected == null && actual == null) {
return;
}
assertEquals(expected.getHits(), actual.getHits());
assertEquals(expected.getMisses(), actual.getMisses());
assertEquals(expected.getEvictions(), actual.getEvictions());
assertEquals(expected.getSizeInBytes(), actual.getSizeInBytes());
assertEquals(expected.getItems(), actual.getItems());
}
}
7 changes: 7 additions & 0 deletions ...c/internalClusterTest/java/org/opensearch/indices/CacheStatsAPIIndicesRequestCacheIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest;
import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse;
import org.opensearch.action.admin.indices.cache.clear.ClearIndicesCacheRequest;
import org.opensearch.action.admin.indices.forcemerge.ForceMergeResponse;
import org.opensearch.action.admin.indices.stats.CommonStatsFlags;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.client.Client;
Expand All @@ -23,12 +24,14 @@
import org.opensearch.common.cache.stats.ImmutableCacheStats;
import org.opensearch.common.cache.stats.ImmutableCacheStatsHolder;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.common.xcontent.XContentHelper;
import org.opensearch.core.xcontent.MediaTypeRegistry;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.cache.request.RequestCacheStats;
import org.opensearch.index.query.QueryBuilders;
import org.opensearch.test.OpenSearchIntegTestCase;
Expand Down Expand Up @@ -266,10 +269,14 @@ private void startIndex(Client client, String indexName) throws InterruptedExcep
.put(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), true)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
// Disable index refreshing to avoid cache being invalidated mid-test
.put(IndexSettings.INDEX_REFRESH_INTERVAL_SETTING.getKey(), TimeValue.timeValueMillis(-1))
)
.get()
);
indexRandom(true, client.prepareIndex(indexName).setSource("k", "hello"));
// Force merge the index to ensure there can be no background merges during the subsequent searches that would invalidate the cache
ForceMergeResponse forceMergeResponse = client.admin().indices().prepareForceMerge(indexName).setFlush(true).get();
ensureSearchable(indexName);
}

Expand Down

0 comments on commit 5cb11af

Please sign in to comment.