Cache sticky endpoint channels by schlosna · Pull Request #2993 · palantir/dialogue

schlosna · 2026-05-15T13:42:28Z

Before this PR

Dialogue clients that make heavy use of sticky channels incur a significant construction overhead (e.g. see palantir/conjure-java#2869 ) leading to per-endpoint TypeMarker
anonymous classes, ExceptionDeserializerArgs instances, and Deserializer fields,
causing excessive allocations on high throughput services.

This adds unnecessary overhead for conjure clients that use sticky channels such as ⛰️ clients via e.g. DataServiceAsync#of(com.palantir.dialogue.EndpointChannelFactory, com.palantir.dialogue.ConjureRuntime).

See #2647 which was a basis for this PR.

After this PR

==COMMIT_MSG==

Summary

Introduce StickyEndpointChannelsFactory interface — replaces the Supplier<Channel> return type from StickyEndpointChannels2.create() and DialogueChannel.stickyChannels() with a typed stickyChannel() method. DialogueChannel now directly implements this interface, and DialogueChannel.stickyChannels() is deprecated in favor of stickyChannel().
Extract CachingEndpointChannelFactory — encapsulates the Caffeine LoadingCache<Endpoint, EndpointChannel> with delegate::endpoint as the loader into a reusable EndpointChannelFactory implementation. Cache size is configurable via Config.maxEndpointCacheSize() (default 1000), and caching is bypassed when set to <= 0.
Simplify StickyEndpointChannels2 internals — removes the StickyEndpointChannels2EndpointFactorySupplier and StickyEndpointChannel inner classes; queue override attachment and sticky routing are now inlined in StickyChannel2.endpoint(), reducing indirection.
Update ReloadingClientFactory — InternalDialogueChannel now extends StickyEndpointChannelsFactory instead of declaring its own stickyChannels() method, and callers use the new stickyChannel() API directly.

Test plan

Existing StickyEndpointChannels2Test passes (caching behavior is identical, just encapsulated)
SimulationTest passes and regenerated simulation resources reflect updated sticky channel behavior
ReloadingClientFactoryTest passes with the new stickyChannel() API

==COMMIT_MSG==

Possible downsides?

This encapsulates a cache of Endpoint -> EndpointChannel so there will be some memory overhead; however, use of sticky channels implies these routes intend to stick around in memory and amortize overhead for benefit of throughput.

changelog-app · 2026-05-15T13:42:32Z

Generate changelog in `changelog/@unreleased`

Type (Select exactly one)

Feature (Adding new functionality)
Improvement (Improving existing functionality)
Fix (Fixing an issue with existing functionality)
Break (Creating a new major version by breaking public APIs)
Deprecation (Removing functionality in a non-breaking way)
Migration (Automatically moving data/functionality to a new system)

Description

Cache sticky endpoint channels

Check the box to generate changelog(s)

Generate changelog entry

changelog-app · 2026-05-22T15:52:40Z

✅ Successfully generated changelog entry!

Need to regenerate?

Simply interact with the changelog bot comment again to regenerate these entries.

📋Changelog Preview

💡 Improvements

Cache sticky endpoint channels (#2993)

aldexis · 2026-05-26T13:10:13Z

+    /** Maximum number of cached endpoint channels. Values {@code <= 0} disable caching. */
+    @Value.Default
+    default int maxEndpointCacheSize() {
+        return 1_000;


Thoughts on starting with this at 0 to keep parity with existing logic, and testing a different value in some prod environment, before changing the default?

I also wonder whether 1000 is enough/too much.

aldexis · 2026-05-26T13:11:04Z

+    /** Maximum number of cached endpoint channels. Values {@code <= 0} disable caching. */
+    @Value.Default
+    default int maxEndpointCacheSize() {


This seems to be specific to sticky channels, but the javadoc doesn't surface that at all. cached endpoint channels is instead pretty generic. It may be worth adding some clarification.

Additionally, I don't think users can modify this config directly, so we may want to pipe it through DialogueChannel.Builder

aldexis · 2026-05-26T13:15:03Z

+        this.cache = Caffeine.newBuilder()
+                .maximumSize(maxEndpointCacheSize)
+                .weakValues()
+                .build(delegate::endpoint);


side-note: it may be worth starting to use https://github.com/palantir/cache for new caches like this (which iirc gives us instrumentation by default, which could be interesting to have regardless, since that would help identify cases where the cache size may be inappropriate)

That library doesn't support weak values (and I'm not sure I want to support weak values there). Using Caffeine feels more appropriate here.

aldexis · 2026-05-26T13:17:11Z

-        @Override
-        public String toString() {
-            return "StickyEndpointChannel{delegate=" + delegate + '}';
-        }


Seeing this makes me wonder whether anybody may have been logging the channel, as a way to debug stuff, and whether the loss of this override may cause debugging pain (since you're now returning request -> router.execute(request, endpointWithQueueOverride);)

pkoenig10 · 2026-05-26T15:26:12Z

+package com.palantir.dialogue.core;
+
+import com.palantir.dialogue.Channel;
+
+public interface StickyEndpointChannelsFactory {


Change the package and make the name non-plural to be consistent with EndpointChannelFactory.

Also this returns Channel, not EndpointChannel.

Suggested change

package com.palantir.dialogue.core;

import com.palantir.dialogue.Channel;

public interface StickyEndpointChannelsFactory {

package com.palantir.dialogue;

import com.palantir.dialogue.Channel;

public interface StickyChannelFactory {

pkoenig10 · 2026-05-26T15:30:27Z

+        int maxEndpointCacheSize = cf.maxEndpointCacheSize();
+        EndpointChannelFactory channelFactory = (maxEndpointCacheSize <= 0)
+                ? delegate
+                : new CachingEndpointChannelFactory(delegate, maxEndpointCacheSize);


This feels a bit like overkill to me.

If we want to keep this, I'd consider making the CachingEndpointChannelFactory constructor private and pushing this logic into a static factory method there, so callers don't need to be concerned with this.

pkoenig10 · 2026-05-26T16:35:55Z

        }
    }
-
-    private static final class StickyEndpointChannel implements EndpointChannel {


nit: Can we keep this? Having non-anonymous classes is helpful for telemetry/debugging because it has a nice name in stacktraces and has a toString implementation.

pkoenig10 · 2026-05-26T16:37:35Z

@@ -50,7 +50,8 @@ public Channel getChannel(Simulation simulation, Supplier<Map<String, Simulation

    public Supplier<Channel> getSticky2NonReloading(Simulation simulation, Map<String, SimulationServer> servers) {


This return type should probably be changed to use the new factory interface we've introduced. Then we can just do return dialogueChannelWithDefaults(...).

pkoenig10 · 2026-05-26T16:38:33Z

-        sticky = StickyEndpointChannels2.create(config, nodeSelectionChannel, endpointChannelFactory);
+        StickyEndpointChannelsFactory stickyEndpointChannelsFactory =
+                StickyEndpointChannels2.create(config, nodeSelectionChannel, endpointChannelFactory);
+        sticky = () -> stickyEndpointChannelsFactory.stickyChannel();


Same here - we should get rid of this Supplier indirection because it just makes the code harder to understand. Tests can just call stickyChannel() instead of get().

aldexis · 2026-06-02T15:01:47Z

@schlosna What is the current status of this PR? We've noticed similar things across a number of services, as the largest allocation pressure (especially on smaller stacks), and I'm keen to figure out if we can reduce/remove this

aldexis · 2026-06-02T15:22:33Z

            @Override
            public <T> T sticky(Class<T> clientInterface) {
                return session().sticky(clientInterface);
            }


@schlosna If I'm correct, our usage looks roughly like

Supplier<DialogueService> serviceSupplier = () -> { DialogueClients.StickyChannelSession stickySession = stickyChannelFactory2.session(); return stickySession.sticky(DialogueService.class); };

and most of the allocations actually from the ExceptionDeserializerArgs that get created in the dialogue clients upon instantiation.

I don't think this PR would actually change that fact, even if we cache the channel, because the client itself would be recreated everytime?

I've added a benchmark in #3021 showing that

Exception mapper make up ~75% of the allocations (probably lower/higher depending on how many errors actually exist - this is for ~100 errors)

Channels make up another ~25%

The rest (including TypeMarker and deserializer allocations) seem to make up ~1.5% of the current allocation profile

Seems like we need both this and palantir/conjure-java#2869 or instead cache the dialogue client creation itself (which maybe we should do? Creating tons of dialogue clients isn't particularly efficient either)

phase1

335f2f3

phase2

a6411a5

schlosna force-pushed the davids/sticky-cache-clean branch 2 times, most recently from ef6dbdb to 0617986 Compare May 15, 2026 14:19

Extract CachingEndpointChannelFactory from StickyEndpointChannels2

860dd5f

schlosna force-pushed the davids/sticky-cache-clean branch from 0617986 to 860dd5f Compare May 15, 2026 14:21

schlosna changed the title ~~Davids/sticky cache clean~~ Cache sticky endpoint channels May 15, 2026

schlosna added 2 commits May 15, 2026 10:33

phase4

7feff96

simulation

7dc86c3

schlosna marked this pull request as ready for review May 22, 2026 15:52

schlosna requested review from a team, aldexis, fsamuel-bs and pkoenig10 May 22, 2026 15:53

aldexis reviewed May 26, 2026

View reviewed changes

pkoenig10 reviewed May 26, 2026

View reviewed changes

aldexis reviewed Jun 2, 2026

View reviewed changes

aldexis mentioned this pull request Jun 8, 2026

Add a benchmark for testing client creation with lots of endpoints and errors #3021

Draft

		@@ -50,7 +50,8 @@ public Channel getChannel(Simulation simulation, Supplier<Map<String, Simulation

		public Supplier<Channel> getSticky2NonReloading(Simulation simulation, Map<String, SimulationServer> servers) {

Uh oh!

Conversation

schlosna commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before this PR

After this PR

Summary

Test plan

Possible downsides?

Uh oh!

changelog-app Bot commented May 15, 2026 • edited by schlosna Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Generate changelog in changelog/@unreleased

Uh oh!

changelog-app Bot commented May 22, 2026

✅ Successfully generated changelog entry!

Need to regenerate?

📋Changelog Preview

💡 Improvements

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkoenig10 May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aldexis commented Jun 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aldexis Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

schlosna commented May 15, 2026 •

edited

Loading

changelog-app Bot commented May 15, 2026 •

edited by schlosna

Loading

Generate changelog in `changelog/@unreleased`

pkoenig10 May 26, 2026 •

edited

Loading

aldexis Jun 8, 2026 •

edited

Loading