Skip to content

Commit

Permalink
[Tiered Caching] Additional ITs for cache stats (opensearch-project#1…
Browse files Browse the repository at this point in the history 
…3655)

* Adds cache clear IT

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

Cleaned up logic for cache stats ITs

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

Adds more tests around tiered spillover cache

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

Fixed cache stats behavior for overall /_nodes/stats call

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

cleanup

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

Fixed folder structure

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

Addressed Sagar's comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

Addressed Ankit's comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

Break horrifyingly long test case into many shorter cases

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

Added unsupported operation exception to TSC stats holder incrementEvictions()

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

Addressed Sorabh's comments

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun assemble

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

* rerun gradle

Signed-off-by: Peter Alfonsi <petealft@amazon.com>

---------

Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Co-authored-by: Peter Alfonsi <petealft@amazon.com>
  • Loading branch information
@peteralfonsi
peteralfonsi and Peter Alfonsi committed Sep 3, 2024
1 parent be97e90 commit 9c4d4af
Show file tree
Hide file tree
Showing 9 changed files with 700 additions and 99 deletions.
4 changes: 2 additions & 2 deletions ...e.common.tier/TieredSpilloverCacheIT.java → ...e/common/tier/TieredSpilloverCacheIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(TieredSpilloverCachePlugin.class, MockDiskCachePlugin.class);
}

private Settings defaultSettings(String onHeapCacheSizeInBytesOrPecentage) {
static Settings defaultSettings(String onHeapCacheSizeInBytesOrPercentage) {
return Settings.builder()
.put(FeatureFlags.PLUGGABLE_CACHE, "true")
.put(
Expand All @@ -88,7 +88,7 @@ private Settings defaultSettings(String onHeapCacheSizeInBytesOrPecentage) {
OpenSearchOnHeapCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE)
.get(MAXIMUM_SIZE_IN_BYTES_KEY)
.getKey(),
onHeapCacheSizeInBytesOrPecentage
onHeapCacheSizeInBytesOrPercentage
)
.build();
}
Expand Down
501 changes: 501 additions & 0 deletions ...nternalClusterTest/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsIT.java
View file

Large diffs are not rendered by default.

16 changes: 12 additions & 4 deletions ...les/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ private Function<ICacheKey<K>, Tuple<V, String>> getValueFromTieredCache(boolean
void handleRemovalFromHeapTier(RemovalNotification<ICacheKey<K>, V> notification) {
ICacheKey<K> key = notification.getKey();
boolean wasEvicted = SPILLOVER_REMOVAL_REASONS.contains(notification.getRemovalReason());
boolean countEvictionTowardsTotal = false; // Don't count this eviction towards the cache's total if it ends up in the disk tier
if (caches.get(diskCache).isEnabled() && wasEvicted && evaluatePolicies(notification.getValue())) {
try (ReleasableLock ignore = writeLock.acquire()) {
diskCache.put(key, notification.getValue()); // spill over to the disk tier and increment its stats
Expand All @@ -336,21 +337,28 @@ void handleRemovalFromHeapTier(RemovalNotification<ICacheKey<K>, V> notification
// If the value is not going to the disk cache, send this notification to the TSC's removal listener
// as the value is leaving the TSC entirely
removalListener.onRemoval(notification);
countEvictionTowardsTotal = true;
}
updateStatsOnRemoval(TIER_DIMENSION_VALUE_ON_HEAP, wasEvicted, key, notification.getValue());
updateStatsOnRemoval(TIER_DIMENSION_VALUE_ON_HEAP, wasEvicted, key, notification.getValue(), countEvictionTowardsTotal);
}

void handleRemovalFromDiskTier(RemovalNotification<ICacheKey<K>, V> notification) {
// Values removed from the disk tier leave the TSC entirely
removalListener.onRemoval(notification);
boolean wasEvicted = SPILLOVER_REMOVAL_REASONS.contains(notification.getRemovalReason());
updateStatsOnRemoval(TIER_DIMENSION_VALUE_DISK, wasEvicted, notification.getKey(), notification.getValue());
updateStatsOnRemoval(TIER_DIMENSION_VALUE_DISK, wasEvicted, notification.getKey(), notification.getValue(), true);
}

void updateStatsOnRemoval(String removedFromTierValue, boolean wasEvicted, ICacheKey<K> key, V value) {
void updateStatsOnRemoval(
String removedFromTierValue,
boolean wasEvicted,
ICacheKey<K> key,
V value,
boolean countEvictionTowardsTotal
) {
List<String> dimensionValues = statsHolder.getDimensionsWithTierValue(key.dimensions, removedFromTierValue);
if (wasEvicted) {
statsHolder.incrementEvictions(dimensionValues);
statsHolder.incrementEvictions(dimensionValues, countEvictionTowardsTotal);
}
statsHolder.decrementItems(dimensionValues);
statsHolder.decrementSizeInBytes(dimensionValues, weigher.applyAsLong(key, value));
Expand Down
29 changes: 19 additions & 10 deletions ...ommon/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCacheStatsHolder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,20 +105,29 @@ public void incrementMisses(List<String> dimensionValues) {
internalIncrement(dimensionValues, missIncrementer, true);
}

/**
* This method shouldn't be used in this class. Instead, use incrementEvictions(dimensionValues, includeInTotal)
* which specifies whether the eviction should be included in the cache's total evictions, or if it should
* just count towards that tier's evictions.
* @param dimensionValues The dimension values
*/
@Override
public void incrementEvictions(List<String> dimensionValues) {
final String tierValue = validateTierDimensionValue(dimensionValues);
throw new UnsupportedOperationException(
"TieredSpilloverCacheHolder must specify whether to include an eviction in the total cache stats. Use incrementEvictions(List<String> dimensionValues, boolean includeInTotal)"
);
}

// If the disk tier is present, only evictions from the disk tier should be included in total values.
/**
* Increment evictions for this set of dimension values.
* @param dimensionValues The dimension values
* @param includeInTotal Whether to include this eviction in the total for the whole cache's evictions
*/
public void incrementEvictions(List<String> dimensionValues, boolean includeInTotal) {
validateTierDimensionValue(dimensionValues);
// If we count this eviction towards the total, we should increment all ancestor nodes. If not, only increment the leaf node.
Consumer<DefaultCacheStatsHolder.Node> evictionsIncrementer = (node) -> {
if (tierValue.equals(TIER_DIMENSION_VALUE_ON_HEAP) && diskCacheEnabled) {
// If on-heap tier, increment only the leaf node corresponding to the on heap values; not the total values in its parent
// nodes
if (node.isAtLowestLevel()) {
node.incrementEvictions();
}
} else {
// If disk tier, or on-heap tier with a disabled disk tier, increment the leaf node and its parents
if (includeInTotal || node.isAtLowestLevel()) {
node.incrementEvictions();
}
};
Expand Down
17 changes: 15 additions & 2 deletions ...ache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -926,14 +926,14 @@ public void testDiskTierPolicies() throws Exception {
MockCacheRemovalListener<String, String> removalListener = new MockCacheRemovalListener<>();
TieredSpilloverCache<String, String> tieredSpilloverCache = intializeTieredSpilloverCache(
keyValueSize,
100,
keyValueSize * 100,
removalListener,
Settings.builder()
.put(
OpenSearchOnHeapCacheSettings.getSettingListForCacheType(CacheType.INDICES_REQUEST_CACHE)
.get(MAXIMUM_SIZE_IN_BYTES_KEY)
.getKey(),
onHeapCacheSize * 50 + "b"
onHeapCacheSize * keyValueSize + "b"
)
.build(),
0,
Expand All @@ -955,6 +955,7 @@ public void testDiskTierPolicies() throws Exception {

LoadAwareCacheLoader<ICacheKey<String>, String> loader = getLoadAwareCacheLoader(keyValuePairs);

int expectedEvictions = 0;
for (String key : keyValuePairs.keySet()) {
ICacheKey<String> iCacheKey = getICacheKey(key);
Boolean expectedOutput = expectedOutputs.get(key);
Expand All @@ -967,8 +968,15 @@ public void testDiskTierPolicies() throws Exception {
} else {
// Should miss as heap tier size = 0 and the policy rejected it
assertNull(result);
expectedEvictions++;
}
}

// We expect values that were evicted from the heap tier and not allowed into the disk tier by the policy
// to count towards total evictions
assertEquals(keyValuePairs.size(), getEvictionsForTier(tieredSpilloverCache, TIER_DIMENSION_VALUE_ON_HEAP));
assertEquals(0, getEvictionsForTier(tieredSpilloverCache, TIER_DIMENSION_VALUE_DISK)); // Disk tier is large enough for no evictions
assertEquals(expectedEvictions, getTotalStatsSnapshot(tieredSpilloverCache).getEvictions());
}

public void testTookTimePolicyFromFactory() throws Exception {
Expand Down Expand Up @@ -1493,6 +1501,11 @@ private ImmutableCacheStats getStatsSnapshotForTier(TieredSpilloverCache<?, ?> t
return snapshot;
}

private ImmutableCacheStats getTotalStatsSnapshot(TieredSpilloverCache<?, ?> tsc) throws IOException {
ImmutableCacheStatsHolder cacheStats = tsc.stats(new String[0]);
return cacheStats.getStatsForDimensionValues(List.of());
}

// Duplicated here from EhcacheDiskCacheTests.java, we can't add a dependency on that plugin
static class StringSerializer implements Serializer<String, byte[]> {
private final Charset charset = StandardCharsets.UTF_8;
Expand Down
Loading

0 comments on commit 9c4d4af

Please sign in to comment.