Skip to content

Commit

Permalink
replace missed caches, drop some instrumentation
Browse files Browse the repository at this point in the history
  • Loading branch information
carterkozak committed Jan 30, 2025
1 parent 5e66e89 commit f093736
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 54 deletions.
1 change: 0 additions & 1 deletion dialogue-clients/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
19 changes: 0 additions & 19 deletions dialogue-clients/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,11 +37,12 @@ final class CachingFallbackDnsResolver implements DialogueDnsResolver {
private final Meter lookupFallback;
private final Meter lookupFailure;

@SuppressWarnings("checkstyle:IllegalType")
private final Cache<String, ImmutableSet<InetAddress>> fallbackCache;

CachingFallbackDnsResolver(DialogueDnsResolver delegate, TaggedMetricRegistry registry) {
this.delegate = delegate;
this.fallbackCache = Caffeine.newBuilder()
this.fallbackCache = CacheBuilder.newBuilder()
.maximumSize(1000)
.expireAfterWrite(Duration.ofMinutes(10))
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -69,11 +70,18 @@ final class ChannelCache {
*/
private final Map<String, ApacheCacheEntry> apacheCache = new ConcurrentHashMap<>();

private final LoadingCache<ChannelCacheKey, DialogueChannel> channelCache = Caffeine.newBuilder()
@SuppressWarnings("checkstyle:IllegalType")
private final LoadingCache<ChannelCacheKey, DialogueChannel> 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() {
Expand Down Expand Up @@ -118,7 +126,7 @@ DialogueChannel getNonReloadingChannel(
@Safe String channelName,
Optional<OverrideHostIndex> 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? {} {} {}",
Expand All @@ -128,7 +136,7 @@ DialogueChannel getNonReloadingChannel(
}
}

return channelCache.get(ImmutableChannelCacheKey.builder()
return channelCache.getUnchecked(ImmutableChannelCacheKey.builder()
.from(reloadingParams)
.blockingExecutor(reloadingParams.blockingExecutor())
.serviceConf(serviceConf)
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String, MaybeUri> uriCache = CacheStats.of(
SharedTaggedMetricRegistries.getSingleton(), "dialogue-uri")
.register(stats -> Caffeine.newBuilder()
.maximumWeight(100_000)
.<String, MaybeUri>weigher((key, _value) -> key.length())
.expireAfterAccess(Duration.ofMinutes(1))
.softValues()
.recordStats(stats)
.build(DnsSupport::tryParseUri));
@SuppressWarnings("checkstyle:IllegalType")
private static final LoadingCache<String, MaybeUri> uriCache = CacheBuilder.newBuilder()
.maximumWeight(100_000)
.<String, MaybeUri>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() {
Expand Down Expand Up @@ -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(
Expand Down
21 changes: 10 additions & 11 deletions versions.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit f093736

Please sign in to comment.