Skip to content

[fix][cli] Restore the 64M default client memory limit in pulsar-perf #26341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AlvaroStream wants to merge 1 commit into
base: master
Choose a base branch
from
+40 −3
Open

[fix][cli] Restore the 64M default client memory limit in pulsar-perf #26341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ClientConfigurationData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
public class ClientConfigurationData implements Serializable, Cloneable {
private static final long serialVersionUID = 1L;

public static final long DEFAULT_MEMORY_LIMIT_BYTES = 64 * 1024 * 1024;

@Schema(
name = "serviceUrl",
requiredMode = Schema.RequiredMode.REQUIRED,
Expand Down Expand Up @@ -433,7 +435,7 @@ public class ClientConfigurationData implements Serializable, Cloneable {
description = "Limit of client memory usage (in byte). The 64M default can guarantee a high producer "
+ "throughput."
)
private long memoryLimitBytes = 64 * 1024 * 1024;
private long memoryLimitBytes = DEFAULT_MEMORY_LIMIT_BYTES;

@Schema(
name = "proxyServiceUrl",
Expand Down
6 changes: 4 additions & 2 deletions pulsar-testclient/src/main/java/org/apache/pulsar/testclient/PerformanceBaseArguments.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.pulsar.testclient;

import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.pulsar.client.impl.conf.ClientConfigurationData.DEFAULT_MEMORY_LIMIT_BYTES;
import org.apache.pulsar.cli.converters.picocli.ByteUnitToLongConverter;
import org.apache.pulsar.client.api.ProxyProtocol;
import picocli.CommandLine.Option;
Expand Down Expand Up @@ -98,8 +99,9 @@ public abstract class PerformanceBaseArguments extends CmdBase{
public String deprecatedAuthPluginClassName;

@Option(names = { "-ml", "--memory-limit", }, description = "Configure the Pulsar client memory limit "
+ "(eg: 32M, 64M)", converter = ByteUnitToLongConverter.class)
public long memoryLimit;
+ "(eg: 32M, 64M). Use 0 to disable the limit. Default: 64M",
converter = ByteUnitToLongConverter.class)
public long memoryLimit = DEFAULT_MEMORY_LIMIT_BYTES;
Comment on lines +102 to +104

@lhotari lhotari Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before changing this, we'd need to find the actual root cause since this change will cause different pulsar-perf results in certain cases. This has happened in the past with changes in #13344. #15723 and #15748 were made at that time to address the performance regression, #15748 has some context.

Although setting the memory limit will bound the memory usage, it changes the results. For users using specific parameters with an older version of the tool will get different results where the backpressure is expected to be applied by maxPendingMessages and maxPendingMessagesAcrossPartitions.

The intention of this change has been to apply backpressure:

pulsar/pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarClientImpl.java

Lines 336 to 341 in 2baa1f7

if (!memoryLimitController.isMemoryLimited()) {
// set default limits for producers when memory limit controller is disabled
producerBuilder.maxPendingMessages(NO_MEMORY_LIMIT_DEFAULT_MAX_PENDING_MESSAGES);
producerBuilder.maxPendingMessagesAcrossPartitions(
NO_MEMORY_LIMIT_DEFAULT_MAX_PENDING_MESSAGES_ACROSS_PARTITIONS);
}

The possible root cause could be that this regresses in some way between 3.0.5 and 4.0.9.

@lhotari lhotari Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I performed analysis with Claude and based on that, it seems that the NO_MEMORY_LIMIT_DEFAULT_MAX_PENDING_MESSAGES / NO_MEMORY_LIMIT_DEFAULT_MAX_PENDING_MESSAGES_ACROSS_PARTITIONS doesn't get applied for non-partitioned topics.

If we change the default memory limit for pulsar-perf, I believe that the limit should be proportional to the max direct memory provided to pulsar-perf. The reason for this is that it would prevent silent performance regressions caused by the parameter change while capping the memory limit and preventing OOM.
Since pulsar-perf doesn't use direct memory for other purposes than Netty, it could use 50% of available direct memory.

Code example of setting a parameter based on available direct memory:

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/ManagedLedgerClientFactory.java

Lines 99 to 103 in 083fd4c

if (managedLedgerMaxReadsInFlightSizeInMB == null) {
// When unset, default to 15% of the available JVM direct memory, but never below the maximum
// size of a single read (dispatcherMaxReadSizeBytes) so that the limiter can never block the
// completion of one read.
long fractionOfDirectMemory = (long) (0.15d * DirectMemoryUtils.jvmMaxDirectMemory());

@lhotari lhotari Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's #26342 which fixes the root cause of #26340. As mentioned in the previous commit, I'd suggest taking the path where instead of setting the memory limit to 64M, it would be 50% of available direct memory. This is something that could also be discussed on the dev mailing list.

@AlvaroStream AlvaroStream Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make it the default so test can be compared and improve it later?

@lhotari lhotari Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make it the default so test can be compared and improve it later?

I'd suggest taking the path where instead of setting the memory limit to 64M, it would be 50% of available direct memory.

This would make pulsar-perf test results more consistent across versions since there hasn't been a limit in the past. Setting a limit changes the behavior significantly for many workloads since there will be less inflight messages. This would mainly impact tests where there are a lot of partitions and the message sizes are relatively large (for example 100 partitions, 32kB message size, very high message rate).
Making the default proportional to the available direct memory is useful since the direct memory would actually get used when there's available memory to use.

The problem with the previous NO_MEMORY_LIMIT_DEFAULT_MAX_PENDING_MESSAGES/NO_MEMORY_LIMIT_DEFAULT_MAX_PENDING_MESSAGES_ACROSS_PARTITIONS solution was that it didn't apply to non-partitioned topics. That's fixed by #26342. Since those limits have been in place for partitioned topics, setting the memory limit to 64M change the behavior and produce different results.

Hopefully this clarifies the reason why setting to 64M isn't something that I support and I'm instead recommending to make it dynamic, based on available direct memory.
It's a very simple change to this PR to make it dynamic. (long) (0.5d * DirectMemoryUtils.jvmMaxDirectMemory()) will return 50% of total direct memory in bytes.

public PerformanceBaseArguments(String cmdName) {
super(cmdName);
}
Expand Down
33 changes: 33 additions & 0 deletions ...r-testclient/src/test/java/org/apache/pulsar/testclient/PerformanceBaseArgumentsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.pulsar.testclient;

import static org.apache.pulsar.client.api.ProxyProtocol.SNI;
import static org.apache.pulsar.client.impl.conf.ClientConfigurationData.DEFAULT_MEMORY_LIMIT_BYTES;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.fail;
import java.io.File;
Expand Down Expand Up @@ -249,6 +250,38 @@ public void run() throws Exception {
// Act
baseArgument.parseCLI();

// Assert
assertEquals(baseArgument.memoryLimit, DEFAULT_MEMORY_LIMIT_BYTES);
}
}

@Test
public void testMemoryLimitCanBeDisabled() throws Exception {
for (String cmd : List.of(
"pulsar-perf read",
"pulsar-perf produce",
"pulsar-perf consume",
"pulsar-perf transaction"
)) {
// Arrange
final PerformanceBaseArguments baseArgument = new PerformanceBaseArguments("") {
@Override
public void run() throws Exception {

}

};
String confFile = "./src/test/resources/perf_client1.conf";
Properties prop = new Properties(System.getProperties());
try (FileInputStream fis = new FileInputStream(confFile)) {
prop.load(fis);
}
baseArgument.getCommander().setDefaultValueProvider(PulsarPerfTestPropertiesProvider.create(prop));
baseArgument.parse(new String[]{"-ml", "0"});

// Act
baseArgument.parseCLI();

// Assert
assertEquals(baseArgument.memoryLimit, 0L);
}
Expand Down