[Bug] Shared MemoryLimitController silently disables the client memory limit and drops the client memory metrics

[lhotari]

Search before asking

• I searched the issues and found nothing similar. The feature — sharing a MemoryLimitController across PulsarClient instances — already exists ([Enhancement] PIP-234: Add a solution to share the memory limit controller solution #25212, closed by [improve][client] PIP-234: Support sharing the memory limit controller across multiple isolated Pulsar client instances #25477). This issue is about defects in that shipped feature.

Problem

PulsarClientSharedResources lets several PulsarClient instances share one MemoryLimitController (PIP-234, #25477). Three defects make it a footgun rather than a safety mechanism:

A. Sharing resources silently disables the memory limit. The shared limit defaults to 0 = unlimited, and an unconfigured shared-resources instance shares everything, including the memory limit controller. So the documented "share all resources" snippet silently removes the 64 MiB default limit from every client using it — in exactly the many-clients scenario where a bound matters most.

B. Per-client ClientBuilder.memoryLimit(...) is silently ignored when a controller is injected. No warning, no exception, no validation. Two clients declaring different limits while sharing: both values are discarded.

C. The two client memory metrics disappear, or land on the wrong meter provider.

Concrete evidence

A — PulsarClientSharedResourcesBuilderImpl.java:209-210:

static class MemoryLimitResourceConfig implements ResourceConfig, MemoryLimitConfig {
    long memoryLimit;                       // defaults to 0

0 means unlimited (MemoryLimitController.isMemoryLimited() → memoryLimit > 0, MemoryLimitController.java:153-155), and an unconfigured builder shares every resource type (PulsarClientSharedResourcesImpl.java:70-71, EnumSet.allOf(SharedResource.class)), which includes SharedResource.MemoryLimitController (:103-106). The injected controller then wins unconditionally — PulsarClientImpl.java:347-354:

if (memoryLimitController == null) {
    this.memoryLimitController = new MemoryLimitController(conf.getMemoryLimitBytes(), ...);
} else {
    this.memoryLimitController = memoryLimitController;      // conf.getMemoryLimitBytes() never read
    this.memoryLimitController.registerTrigger(this.memoryLimitTrigger);
}

So ClientConfigurationData.java:436 (memoryLimitBytes = 64 * 1024 * 1024) is discarded. The affected snippet is the documented usage example in PulsarClientSharedResources.java ("To share all possible resources across multiple PulsarClient instances" → builder().build()), and PulsarClientSharedResourcesBuilderImplTest exercises exactly that shape across 1000 clients while asserting nothing about the limit — rg 'MemoryLimit|memoryLimit' over that test file returns zero matches.

B — same PulsarClientImpl.java:347-354. Neither ClientBuilder.memoryLimit(...) nor ClientBuilder.sharedResources(...) javadoc mentions the interaction, and nothing validates a conflict.

C — PulsarClientImpl.java:356-360:

// Only create memory buffer metrics if memory limit controller is local and memory limiting is enabled.
if (memoryLimitController == null && this.memoryLimitController.isMemoryLimited()) {
    this.memoryBufferStats = new MemoryBufferStats(this.instrumentProvider, this.memoryLimitController);
} else {
    this.memoryBufferStats = null;
}

The condition keys off the injected constructor parameter, so any shared controller suppresses the per-client registration of pulsar.client.memory.buffer.usage and pulsar.client.memory.buffer.limit (metrics/MemoryBufferStats.java:26-30). The compensating registration in PulsarClientSharedResourcesImpl.java:112-117 requires both a non-zero shared limit and SharedResource.OpenTelemetry in the shared set. Outcomes:

• Shared controller left at its 0 default → neither side registers → both metrics silently vanish.
• SharedResource.OpenTelemetry not shared (e.g. an explicit resource list, or shareConfigured() with only configureMemoryLimitController(...)) → instrumentProvider == null (:107-110) → both metrics silently vanish, even with a non-zero shared limit.
• Everything shared, limit configured, but OTel unconfigured → the shared InstrumentProvider falls back to GlobalOpenTelemetry.get() (InstrumentProvider.java:38-43), so these two metrics go to the global instance while every other client metric goes to the SDK passed to ClientBuilder.openTelemetry(sdk).

The last case is an API gap, not a missing line. PulsarClientSharedResourcesImpl.applyTo (:168-193) sets 8 things and instrumentProvider is not among them, and it cannot be: the Lombok @Builder sits on the PulsarClientImpl constructor, whose parameter list has no instrumentProvider, so PulsarClientImplBuilder has no such setter. PulsarClientImpl.java:275 unconditionally does new InstrumentProvider(conf.getOpenTelemetry()). Consequently SharedResource.OpenTelemetry affects only the shared memoryBufferStats and nothing else — and repo-wide, configureOpenTelemetry(...) and PulsarClientSharedResourcesImpl.getInstrumentProvider() have zero callers, so nothing today depends on the current behaviour.

MemoryLimitConfig's javadoc reinforces the mismatch: it says "See also ClientBuilder#memoryLimit(long, SizeUnit)", reading as a shared analogue of the client-level setting, while in fact it silently overrides it and only its non-zero form keeps the metrics alive.

Reachability

No broker or network needed. PulsarClientSharedResources.builder().build() + PulsarClient.builder().sharedResources(shared) is sufficient to observe A and C; assert on ((PulsarClientImpl) client).getMemoryLimitController().isMemoryLimited() and on the absence of the two metrics. Existing coverage (ProducerMemoryLimitTest.testMultiPulsarClientProducerShareMemoryLimitController, ConsumerMemoryLimitTest.testMultiPulsarClientConsumerShareMemoryLimitController) only covers the configured, non-zero path.

Proposed solution

For A — pick one; this needs a call:

1. Default the shared limit to ClientConfigurationData's 64 MiB instead of 0. Consistent with the per-client default; changes behaviour for anyone relying on today's accidental "unlimited".
2. Require an explicit configureMemoryLimitController(...) whenever SharedResource.MemoryLimitController is in the shared set, and fail the build otherwise. Loudest and safest; breaks the documented builder().build() example.
3. Exclude MemoryLimitController from the share-everything default so unconfigured sharing leaves each client's own limit intact. Least disruptive, but makes "all" not actually mean all.

For B — reject or warn on a conflict: if a client sets memoryLimit(...) explicitly and is given a shared controller, log a warning (or fail the build) instead of discarding the value silently. Document the precedence on both ClientBuilder.memoryLimit and ClientBuilder.sharedResources.

For C — register the two metrics exactly once regardless of the sharing shape, and route them through the same InstrumentProvider as the rest of the client's metrics. The straightforward version adds instrumentProvider to PulsarClientImpl's builder and passes the shared one through applyTo — a (package-private) API change, hence a design decision rather than a patch. A narrower fix: register MemoryBufferStats on the shared object whenever the shared controller is memory-limited, independent of whether SharedResource.OpenTelemetry is shared.

Fixing A is a prerequisite for the broker/proxy work in #26346: wiring the broker onto a shared controller while that controller silently defaults to unlimited would achieve nothing.

Scope & compatibility

• A changes observable behaviour for anyone already sharing resources without configuring a limit. Framed as a bug fix (the current behaviour silently discards a documented default), but options 1 and 2 cross into "semantics of existing functionality" — flag in release notes; a PIP is arguably warranted for option 2, since it makes an existing documented snippet throw.
• B is validation plus javadoc — bug-fix scope.
• C is a bug fix if it only restores the two existing metric names on the shared path; it needs a PIP if it adds public API (propagating a shared InstrumentProvider into clients).
• No wire-protocol, metadata-format or client-server compatibility impact. The two metric names are unchanged in every option.

Related

• [Enhancement] PIP-234: Add a solution to share the memory limit controller solution #25212 (closed, completed) / [improve][client] PIP-234: Support sharing the memory limit controller across multiple isolated Pulsar client instances #25477 (merged) — PIP-234 shared MemoryLimitController; this issue reports defects in that feature, not the feature itself.
• PIP-234: Support using shared thread pool across multiple Pulsar client instance  #19074 — PIP-234 umbrella; [feat][client] Implement PIP-234 for sharing thread pools and DNS resolver/cache across multiple Pulsar Client instances #24790 — shared thread pools and DNS resolver; [feat][client] PIP-234: Support shared resources in PulsarAdmin to reduce thread usage #24893 / [feat] Pulsar Admin Client should support PIP-234 PulsarClientSharedResources for event loop, timer and DNS resolver sharing #24796 — shared resources in PulsarAdmin; [feat][client] PIP-234: Support shared resources in AuthenticationOAuth2 to reduce thread usage #25072 / [feat] Authentication implementations such as AuthenticationOAuth2 should support sharing thread pools or DNS cache #24795 — shared resources in AuthenticationOAuth2.
• [Bug][Java client] Possible race condition in closing consumer which causes a leak in memory limit controller and direct memory with pooled messages #18938, [Bug][Java client] Possible race condition in consumer's clearIncomingMessages which causes a leak in memory limit controller and direct memory with pooled messages #18939 — memory-limit-controller accounting leaks on consumer close / clearIncomingMessages (different bug, non-overlapping).
• [Bug] Pulsar-perf producer OutOfDirectMemoryError with slow broker #26340 / [fix][cli] Restore the 64M default client memory limit in pulsar-perf #26341 / [fix][client] Apply no-memory-limit producer queue defaults at producer creation #26342 — the client memory-limit default work that surfaced this area.
• [Enhancement] Configurable memory limit for broker, proxy and WebSocket proxy Pulsar clients, shared across all of them #26346 — configurable memory limit for broker, proxy and WebSocket proxy clients (depends on this).

[lhotari]

@oneby-wang would you like to address this as a follow up of #25477?