From 78ac11a3daf7d53122823a7a5858371d924d514e Mon Sep 17 00:00:00 2001 From: Gabor Kaszab Date: Fri, 25 Oct 2024 17:42:09 +0200 Subject: [PATCH] Core: Expose the stats of the manifest file content cache For observability purposes clients could use the stats of the manifest file content cache to see for instance the cache hit/miss ratio so that users can fine tune the configuration of the cache. --- .../org/apache/iceberg/ManifestFiles.java | 6 ++ .../iceberg/metrics/CacheMetricsReport.java | 39 ++++++++++++ .../apache/iceberg/TestManifestCaching.java | 14 +++-- .../metrics/TestCacheMetricsReport.java | 62 +++++++++++++++++++ 4 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 core/src/main/java/org/apache/iceberg/metrics/CacheMetricsReport.java create mode 100644 core/src/test/java/org/apache/iceberg/metrics/TestCacheMetricsReport.java diff --git a/core/src/main/java/org/apache/iceberg/ManifestFiles.java b/core/src/main/java/org/apache/iceberg/ManifestFiles.java index 739f0be251df..86c4e680673b 100644 --- a/core/src/main/java/org/apache/iceberg/ManifestFiles.java +++ b/core/src/main/java/org/apache/iceberg/ManifestFiles.java @@ -33,6 +33,7 @@ import org.apache.iceberg.io.FileIO; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.io.OutputFile; +import org.apache.iceberg.metrics.CacheMetricsReport; import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; @@ -86,6 +87,11 @@ public static void dropCache(FileIO fileIO) { CONTENT_CACHES.cleanUp(); } + /** Get statistics of the manifest file content cache for a FileIO. */ + public static CacheMetricsReport contentCacheStats(FileIO io) { + return CacheMetricsReport.of(contentCache(io).stats()); + } + /** * Returns a {@link CloseableIterable} of file paths in the {@link ManifestFile}. * diff --git a/core/src/main/java/org/apache/iceberg/metrics/CacheMetricsReport.java b/core/src/main/java/org/apache/iceberg/metrics/CacheMetricsReport.java new file mode 100644 index 000000000000..7ea6961857f4 --- /dev/null +++ b/core/src/main/java/org/apache/iceberg/metrics/CacheMetricsReport.java @@ -0,0 +1,39 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.metrics; + +import com.github.benmanes.caffeine.cache.stats.CacheStats; +import org.immutables.value.Value; + +@Value.Immutable +public abstract class CacheMetricsReport implements MetricsReport { + public abstract long hitCount(); + + public abstract long missCount(); + + public abstract long evictionCount(); + + public static CacheMetricsReport of(CacheStats stats) { + return ImmutableCacheMetricsReport.builder() + .hitCount(stats.hitCount()) + .missCount(stats.missCount()) + .evictionCount(stats.evictionCount()) + .build(); + } +} diff --git a/core/src/test/java/org/apache/iceberg/TestManifestCaching.java b/core/src/test/java/org/apache/iceberg/TestManifestCaching.java index 9a944c1583d0..57acb7d92d05 100644 --- a/core/src/test/java/org/apache/iceberg/TestManifestCaching.java +++ b/core/src/test/java/org/apache/iceberg/TestManifestCaching.java @@ -78,16 +78,16 @@ public void testPlanWithCache() throws Exception { assertThat(cache.estimatedCacheSize()) .as("All manifest files should be cached") .isEqualTo(numFiles); - assertThat(cache.stats().loadCount()) + assertThat(cache.stats().loadSuccessCount()) .as("All manifest files should be recently loaded") .isEqualTo(numFiles); - long missCount = cache.stats().missCount(); + long missCount = ManifestFiles.contentCacheStats(table.io()).missCount(); // planFiles and verify that cache size still the same TableScan scan2 = table.newScan(); assertThat(scan2.planFiles()).hasSize(numFiles); assertThat(cache.estimatedCacheSize()).isEqualTo(numFiles); - assertThat(cache.stats().missCount()) + assertThat(ManifestFiles.contentCacheStats(table.io()).missCount()) .as("All manifest file reads should hit cache") .isEqualTo(missCount); @@ -115,10 +115,14 @@ public void testPlanWithSmallCache() throws Exception { assertThat(cache.maxTotalBytes()).isEqualTo(1); assertThat(scan.planFiles()).hasSize(numFiles); assertThat(cache.estimatedCacheSize()).isEqualTo(0); - assertThat(cache.stats().loadCount()) + assertThat(cache.stats().loadSuccessCount()) .as("File should not be loaded through cache") .isEqualTo(0); - assertThat(cache.stats().requestCount()).as("Cache should not serve file").isEqualTo(0); + assertThat( + ManifestFiles.contentCacheStats(table.io()).hitCount() + + ManifestFiles.contentCacheStats(table.io()).missCount()) + .as("Cache should not serve file") + .isEqualTo(0); ManifestFiles.dropCache(scan.table().io()); } diff --git a/core/src/test/java/org/apache/iceberg/metrics/TestCacheMetricsReport.java b/core/src/test/java/org/apache/iceberg/metrics/TestCacheMetricsReport.java new file mode 100644 index 000000000000..bbaca531d68a --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/metrics/TestCacheMetricsReport.java @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.metrics; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.Weigher; +import org.junit.jupiter.api.Test; + +public class TestCacheMetricsReport { + @Test + public void testNoInputStats() { + CacheMetricsReport cacheMetrics = CacheMetricsReport.of(Caffeine.newBuilder().build().stats()); + + assertThat(cacheMetrics.hitCount()).isZero(); + assertThat(cacheMetrics.missCount()).isZero(); + assertThat(cacheMetrics.evictionCount()).isZero(); + } + + @Test + public void testCacheMetricsFromCaffeineCache() { + int maxTotalWeight = 300; + + Cache inputCache = + Caffeine.newBuilder() + .maximumWeight(maxTotalWeight) + .weigher((Weigher) (key, value) -> value * 100) + .recordStats() + .build(); + + inputCache.get(1, key -> key); + inputCache.get(1, key -> key); + inputCache.get(2, key -> key); + inputCache.get(3, key -> key); // This evicts the other entries due to max weight + + inputCache.cleanUp(); + + CacheMetricsReport cacheMetrics = CacheMetricsReport.of(inputCache.stats()); + + assertThat(cacheMetrics.hitCount()).isOne(); + assertThat(cacheMetrics.missCount()).isEqualTo(3); + assertThat(cacheMetrics.evictionCount()).isEqualTo(2); + } +}