From f093736e9ef86ca3d3dbcafc49bd6cd2e3d8fdd3 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 30 Jan 2025 12:37:18 -0500 Subject: [PATCH] replace missed caches, drop some instrumentation --- dialogue-clients/build.gradle | 1 - dialogue-clients/metrics.md | 19 ------------ .../clients/CachingFallbackDnsResolver.java | 7 +++-- .../dialogue/clients/ChannelCache.java | 22 +++++++++----- .../palantir/dialogue/clients/DnsSupport.java | 30 +++++++++++-------- versions.lock | 21 +++++++------ 6 files changed, 46 insertions(+), 54 deletions(-) diff --git a/dialogue-clients/build.gradle b/dialogue-clients/build.gradle index f924883b9..14cff8a07 100644 --- a/dialogue-clients/build.gradle +++ b/dialogue-clients/build.gradle @@ -20,7 +20,6 @@ dependencies { implementation 'com.palantir.safe-logging:logger' implementation 'com.palantir.safe-logging:preconditions' implementation 'com.palantir.safe-logging:safe-logging' - implementation 'com.palantir.tritium:tritium-caffeine' implementation 'com.palantir.tritium:tritium-registry' implementation 'com.palantir.tritium:tritium-metrics' implementation 'io.dropwizard.metrics:metrics-core' diff --git a/dialogue-clients/metrics.md b/dialogue-clients/metrics.md index 9abf908d2..18a24e073 100644 --- a/dialogue-clients/metrics.md +++ b/dialogue-clients/metrics.md @@ -134,25 +134,6 @@ Instrumentation for the ROUND_ROBIN node selection strategy (currently implement Metrics produced instrumented Jackson components. - `json.parser.string.length` tagged `format` (histogram): Histogram describing the length of strings parsed from input. -## Tritium Caffeine - -`com.palantir.tritium:tritium-caffeine` - -### cache -Cache statistic metrics -- `cache.request` (meter): Count of cache requests - - `cache` - - `result` values (`hit`,`miss`) -- `cache.load` (timer): Count of successful cache loads - - `cache` - - `result` values (`success`,`failure`) -- `cache.eviction` tagged `cache`, `cause` (meter): Count of evicted entries -- `cache.eviction.weight` tagged `cache`, `cause` (meter): Count of evicted weights -- `cache.estimated.size` tagged `cache` (gauge): Approximate number of entries in this cache -- `cache.weighted.size` tagged `cache` (gauge): Approximate accumulated weight of entries in this cache -- `cache.maximum.size` tagged `cache` (gauge): Maximum number of cache entries cache can hold if limited -- `cache.stats.disabled` tagged `cache` (meter): Meter marked when `CaffeineCacheStats.registerCache` is called on a cache that does not record stats using `caffeineBuilder.recordStats()`. - ## Tritium Metrics `com.palantir.tritium:tritium-metrics` diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/CachingFallbackDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/CachingFallbackDnsResolver.java index c846c9dc4..23cf9c5ad 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/CachingFallbackDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/CachingFallbackDnsResolver.java @@ -17,8 +17,8 @@ package com.palantir.dialogue.clients; import com.codahale.metrics.Meter; -import com.github.benmanes.caffeine.cache.Cache; -import com.github.benmanes.caffeine.cache.Caffeine; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.common.collect.ImmutableSet; import com.palantir.dialogue.clients.ClientDnsMetrics.Lookup_Result; import com.palantir.dialogue.core.DialogueDnsResolver; @@ -37,11 +37,12 @@ final class CachingFallbackDnsResolver implements DialogueDnsResolver { private final Meter lookupFallback; private final Meter lookupFailure; + @SuppressWarnings("checkstyle:IllegalType") private final Cache> fallbackCache; CachingFallbackDnsResolver(DialogueDnsResolver delegate, TaggedMetricRegistry registry) { this.delegate = delegate; - this.fallbackCache = Caffeine.newBuilder() + this.fallbackCache = CacheBuilder.newBuilder() .maximumSize(1000) .expireAfterWrite(Duration.ofMinutes(10)) .build(); diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ChannelCache.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ChannelCache.java index f9c94c344..e8bf2ece0 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ChannelCache.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ChannelCache.java @@ -16,9 +16,10 @@ package com.palantir.dialogue.clients; -import com.github.benmanes.caffeine.cache.Caffeine; -import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.annotations.VisibleForTesting; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import com.palantir.conjure.java.api.config.service.ServiceConfiguration; import com.palantir.conjure.java.client.config.ClientConfiguration; import com.palantir.dialogue.core.DialogueChannel; @@ -69,11 +70,18 @@ final class ChannelCache { */ private final Map apacheCache = new ConcurrentHashMap<>(); - private final LoadingCache channelCache = Caffeine.newBuilder() + @SuppressWarnings("checkstyle:IllegalType") + private final LoadingCache channelCache = CacheBuilder.newBuilder() .maximumSize(MAX_CACHED_CHANNELS) // Avoid holding onto old targets, which is now more common as we bind to resolved IP addresses .weakValues() - .build(this::createNonLiveReloadingChannel); + .build(new CacheLoader<>() { + @Override + public DialogueChannel load(ChannelCacheKey key) { + return createNonLiveReloadingChannel(key); + } + }); + private final int instanceNumber; private ChannelCache() { @@ -118,7 +126,7 @@ DialogueChannel getNonReloadingChannel( @Safe String channelName, Optional overrideHostIndex) { if (log.isWarnEnabled()) { - long estimatedSize = channelCache.estimatedSize(); + long estimatedSize = channelCache.size(); if (estimatedSize >= MAX_CACHED_CHANNELS * 0.75) { log.warn( "channelCache nearing capacity - possible bug? {} {} {}", @@ -128,7 +136,7 @@ DialogueChannel getNonReloadingChannel( } } - return channelCache.get(ImmutableChannelCacheKey.builder() + return channelCache.getUnchecked(ImmutableChannelCacheKey.builder() .from(reloadingParams) .blockingExecutor(reloadingParams.blockingExecutor()) .serviceConf(serviceConf) @@ -249,7 +257,7 @@ public String toString() { + ", apacheCache.size=" + apacheCache.size() // Channel names are safe-loggable + ", apacheCache=" + apacheCache.keySet() - + ", channelCache.size=" + channelCache.estimatedSize() + "/" + MAX_CACHED_CHANNELS + + ", channelCache.size=" + channelCache.size() + "/" + MAX_CACHED_CHANNELS + ", channelCache=" // Channel names are safe-loggable + channelCache.asMap().keySet().stream() diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DnsSupport.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DnsSupport.java index aac74707b..0d13d30c0 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DnsSupport.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DnsSupport.java @@ -18,10 +18,11 @@ import com.codahale.metrics.Counter; import com.codahale.metrics.Timer; -import com.github.benmanes.caffeine.cache.Caffeine; -import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Suppliers; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -44,7 +45,6 @@ import com.palantir.refreshable.Refreshable; import com.palantir.refreshable.SettableRefreshable; import com.palantir.tritium.metrics.MetricRegistries; -import com.palantir.tritium.metrics.caffeine.CacheStats; import com.palantir.tritium.metrics.registry.SharedTaggedMetricRegistries; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.lang.ref.Cleaner; @@ -93,15 +93,19 @@ final class DnsSupport { /** * Shared cache of string to parsed URI. This avoids excessive allocation overhead when parsing repeated targets. */ - private static final LoadingCache uriCache = CacheStats.of( - SharedTaggedMetricRegistries.getSingleton(), "dialogue-uri") - .register(stats -> Caffeine.newBuilder() - .maximumWeight(100_000) - .weigher((key, _value) -> key.length()) - .expireAfterAccess(Duration.ofMinutes(1)) - .softValues() - .recordStats(stats) - .build(DnsSupport::tryParseUri)); + @SuppressWarnings("checkstyle:IllegalType") + private static final LoadingCache uriCache = CacheBuilder.newBuilder() + .maximumWeight(100_000) + .weigher((key, _value) -> key.length()) + .expireAfterAccess(Duration.ofMinutes(1)) + .softValues() + .recordStats() + .build(new CacheLoader<>() { + @Override + public MaybeUri load(String key) { + return DnsSupport.tryParseUri(key); + } + }); @VisibleForTesting static void invalidateCaches() { @@ -223,7 +227,7 @@ static MaybeUri tryParseUri(@Unsafe String uriString) { @Nullable private static URI tryParseUri(TaggedMetricRegistry metrics, @Safe String serviceName, @Unsafe String uri) { try { - MaybeUri maybeUri = uriCache.get(uri); + MaybeUri maybeUri = uriCache.getUnchecked(uri); URI result = maybeUri.uriOrThrow(); if (result.getHost() == null) { log.error( diff --git a/versions.lock b/versions.lock index d315b40c8..e04fe09f9 100644 --- a/versions.lock +++ b/versions.lock @@ -8,12 +8,12 @@ com.fasterxml.jackson.datatype:jackson-datatype-guava:2.18.2 (1 constraints: 831 com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.18.2 (1 constraints: 831caaa4) com.fasterxml.jackson.datatype:jackson-datatype-joda:2.18.2 (1 constraints: 831caaa4) com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.18.2 (1 constraints: 831caaa4) -com.github.ben-manes.caffeine:caffeine:3.2.0 (3 constraints: 08289433) +com.github.ben-manes.caffeine:caffeine:3.2.0 (2 constraints: bb17f94d) com.google.auto:auto-common:1.2.1 (1 constraints: 17120ffb) com.google.code.findbugs:jsr305:3.0.2 (14 constraints: 49e26143) -com.google.errorprone:error_prone_annotations:2.7.1 (18 constraints: 8420df82) +com.google.errorprone:error_prone_annotations:2.7.1 (17 constraints: 07102aec) com.google.guava:failureaccess:1.0.2 (1 constraints: 150ae2b4) -com.google.guava:guava:33.4.0-jre (18 constraints: 603f58d1) +com.google.guava:guava:33.4.0-jre (17 constraints: 772d73cf) com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava (1 constraints: bd17c918) com.google.j2objc:j2objc-annotations:3.0.0 (1 constraints: 150aeab4) com.palantir.common:streams:2.4.0 (1 constraints: 08050136) @@ -30,29 +30,28 @@ com.palantir.javapoet:javapoet:0.6.0 (2 constraints: c81064d1) com.palantir.nylon:nylon-threads:0.4.0 (1 constraints: 0c10fa91) com.palantir.refreshable:refreshable:2.5.0 (2 constraints: f1188fb2) com.palantir.ri:resource-identifier:2.8.0 (2 constraints: fc1495b7) -com.palantir.safe-logging:logger:3.7.0 (13 constraints: 99d0b80e) +com.palantir.safe-logging:logger:3.7.0 (12 constraints: 4ec09cd5) com.palantir.safe-logging:logger-slf4j:3.7.0 (1 constraints: 050e6842) com.palantir.safe-logging:logger-spi:3.7.0 (2 constraints: 191ea27b) -com.palantir.safe-logging:preconditions:3.7.0 (19 constraints: 4e351810) -com.palantir.safe-logging:safe-logging:3.7.0 (18 constraints: da26b0dc) +com.palantir.safe-logging:preconditions:3.7.0 (18 constraints: 03250acd) +com.palantir.safe-logging:safe-logging:3.7.0 (17 constraints: 8f168d3a) com.palantir.safethreadlocalrandom:safe-thread-local-random:0.3.0 (1 constraints: 0505f435) com.palantir.tokens:auth-tokens:3.18.0 (3 constraints: 2628868e) com.palantir.tracing:tracing:6.20.0 (2 constraints: 5416db0d) com.palantir.tracing:tracing-api:6.20.0 (2 constraints: 0912eb17) com.palantir.tritium:tritium-api:0.96.0 (2 constraints: 3f1f48be) -com.palantir.tritium:tritium-caffeine:0.96.0 (1 constraints: 4105553b) com.palantir.tritium:tritium-core:0.96.0 (1 constraints: 47105ea2) com.palantir.tritium:tritium-ids:0.96.0 (1 constraints: d20fb696) -com.palantir.tritium:tritium-metrics:0.96.0 (2 constraints: c11598e4) -com.palantir.tritium:tritium-registry:0.96.0 (5 constraints: ab561c1f) +com.palantir.tritium:tritium-metrics:0.96.0 (1 constraints: 4105553b) +com.palantir.tritium:tritium-registry:0.96.0 (4 constraints: 2b46fa8f) com.squareup:javapoet:1.13.0 (1 constraints: f50b65f7) -io.dropwizard.metrics:metrics-core:4.2.30 (5 constraints: 5553f0f5) +io.dropwizard.metrics:metrics-core:4.2.30 (4 constraints: d3425911) javax.annotation:javax.annotation-api:1.3.2 (1 constraints: 0805fb35) joda-time:joda-time:2.12.7 (1 constraints: 2f16b1f1) org.apache.httpcomponents.client5:httpclient5:5.3.1 (1 constraints: 0b050e36) org.apache.httpcomponents.core5:httpcore5:5.3.3 (3 constraints: a42a81aa) org.apache.httpcomponents.core5:httpcore5-h2:5.3.3 (1 constraints: 3f130d3c) -org.checkerframework:checker-qual:3.43.0 (4 constraints: a237011e) +org.checkerframework:checker-qual:3.43.0 (3 constraints: 2727263b) org.derive4j:derive4j-annotation:1.1.1 (1 constraints: 0505f435) org.eclipse.collections:eclipse-collections:11.1.0 (1 constraints: 1b108aa9) org.eclipse.collections:eclipse-collections-api:11.1.0 (2 constraints: f8229c26)