[client] Introduce `batchingMaxBytes` setting in pulsar producer #5045

sijie · 2019-08-26T22:52:36Z

Motivation

The message size can vary between applications. Using number of messages to estimate
the resources used for batching leads to unpredictability.

This pull request introduces a new setting batchingMaxBytes in pulsar producer.
It allows applications planning the resource usage for batching in a better way.

*Motivation* The message size can vary between applications. Using number of messages to estimate the resources used for batching leads to unpredictability. This pull request introduces a new setting `batchingMaxBytes` in pulsar producer. It allows applications planning the resource usage for batching in a better way.

merlimat · 2019-08-26T22:58:55Z

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

@@ -212,6 +212,12 @@ private ProducerBuilderImpl(PulsarClientImpl client, ProducerConfigurationData c
        return this;
    }

+    @Override
+    public ProducerBuilder<T> batchingMaxBytes(int batchingMaxBytes) {
+        conf.setBatchingMaxBytes(batchingMaxBytes);


I'd say to do validation check here for > 0 and < MAX_FRAME_SIZE (depending on the max frame size configured)

I removed the check for >0 to allow users choosing the right batching strategy.

I will add the check for MAX_FRAME_SIZE.

merlimat · 2019-08-26T23:00:31Z

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

@@ -83,5 +89,9 @@ public void setProducer(ProducerImpl<?> producer) {
                .convertToWireProtocol(producer.getConfiguration().getCompressionType());
        this.compressor = CompressionCodecProvider.getCompressionCodec(compressionType);
        this.maxNumMessagesInBatch = producer.getConfiguration().getBatchingMaxMessages();
+        this.maxBytesInBatch = producer.getConfiguration().getBatchingMaxBytes();
+        if (this.maxBytesInBatch <= 0) {
+            this.maxBytesInBatch = MAX_MESSAGE_BATCH_SIZE_BYTES;


The constant MAX_MESSAGE_BATCH_SIZE_BYTES is set to 128 KB which is now different from the default max size (1MB).

In any case, I'd prefer the sanity check to be in the builder itself

didn't realize MAX_MESSAGE_BATCH_SIZE_BYTES is 128KB. will fix

sijie · 2019-08-26T23:49:45Z

@merlimat

I fixed the batch container check logic for handling max message size. although I didn't add any validations in producer configuration data. because:

<=0 is used for disabling number-based or size-based batching strategy.
it is hard to get the upper bound for max message size because it is a dynamic number. in that sense, if I configure the max message size to 10MB and maxBytes should be able to set to be more than 5MB but it should be capped to 10MB.

codelipenghui · 2019-08-27T02:20:00Z

pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ProducerBuilder.java

     */
    ProducerBuilder<T> batchingMaxMessages(int batchMessagesMaxMessagesPerBatch);

+    /**
+     * Set the maximum number of bytes permitted in a batch. <i>default: 1MB</i>


Suggested change

* Set the maximum number of bytes permitted in a batch. <i>default: 1MB</i>

* Set the maximum number of bytes permitted in a batch. <i>default: 128KB</i>

codelipenghui · 2019-08-27T02:21:47Z

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

@@ -212,6 +212,12 @@ private ProducerBuilderImpl(PulsarClientImpl client, ProducerConfigurationData c
        return this;
    }

+    @Override
+    public ProducerBuilder<T> batchingMaxBytes(int batchingMaxBytes) {
+        conf.setBatchingMaxBytes(batchingMaxBytes);


Add check for batchingMaxBytes <= ClientCnx.getMaxMessageSize().

We can't do that. Because the max message size is only set when a client connects to a broker.

I explained that case in previous comment.

Oh I see, max message size can be set at broker side.

codelipenghui · 2019-08-27T02:42:11Z

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

    protected int numMessagesInBatch = 0;
    protected long currentBatchSizeBytes = 0;

-    protected static final int INITIAL_BATCH_BUFFER_SIZE = 1024;
-    protected static final int MAX_MESSAGE_BATCH_SIZE_BYTES = 128 * 1024;
+    protected static final int INITIAL_BATCH_BUFFER_SIZE = 128 * 1024;


It's better to keep INITIAL_BATCH_BUFFER_SIZE as 1KB, in key_based batcher properly create lot of ByteBuf, if the INITIAL_BATCH_BUFFER_SIZE is 128KB, more memory overhead may be incurred.

We can add the option for init batch buffer size in the future, so that user can decide according to their own situation.

Good point. I will probably just assign different values for different containers

Yes, initially we were using the max batch size for this buffer, but it was using a huge amount of memory, especially when publishing on a partitioned topic.

The buffer size is self-adapting to the needed usage (eg. it will keep using the same buffer size), so I don't think we need to make it configurable.

codelipenghui · 2019-09-05T07:21:41Z

run cpp tests
run java8 tests

jiazhai · 2019-09-09T02:01:46Z

run java8 tests

aahmed-se · 2019-09-09T03:46:10Z

run java8 tests

sijie · 2019-09-09T20:24:58Z

@codelipenghui @jiazhai @merlimat I have addressed all the comments. PTAL

sijie · 2019-09-16T10:56:50Z

retest this please

sijie · 2019-09-16T23:26:33Z

ping @jiazhai @codelipenghui @merlimat

// run cpp tests
// run java8 tests

codelipenghui

Looks good to me

codelipenghui · 2019-09-17T04:05:24Z

run cpp tests
run java8 tests

sijie · 2019-09-19T12:27:06Z

run java8 tests

sijie · 2019-09-22T00:38:34Z

run java8 tests

jiazhai · 2019-09-28T03:34:06Z

run java8 tests

jiazhai · 2019-10-07T19:36:51Z

run java8 tests

jiazhai · 2019-10-15T07:13:28Z

run java8 tests

merlimat

👍

codelipenghui · 2019-11-10T14:34:34Z

retest this please

codelipenghui · 2019-11-11T01:59:06Z

@sijie Please take a look the tests.

…ching_bytes

sijie · 2019-11-12T15:34:11Z

run java8 tests

sijie added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/client labels Aug 26, 2019

sijie added this to the 2.5.0 milestone Aug 26, 2019

sijie requested review from merlimat, jiazhai and codelipenghui August 26, 2019 22:52

sijie self-assigned this Aug 26, 2019

sijie requested a review from wolfstudy August 26, 2019 22:52

merlimat reviewed Aug 26, 2019

View reviewed changes

sijie added 2 commits August 26, 2019 16:38

Address review comments

e753f16

Use MaxMessageSize

3411e1e

Use MaxMessageSize as the cap for the buffer size

fe65d54

codelipenghui reviewed Aug 27, 2019

View reviewed changes

sijie added 2 commits September 9, 2019 13:20

Merge remote-tracking branch 'apache/master' into add_batching_bytes

2cf91e6

Address review comments

66f89da

codelipenghui approved these changes Sep 17, 2019

View reviewed changes

Merge remote-tracking branch 'apache/master' into add_batching_bytes

b225ca2

jiazhai approved these changes Sep 28, 2019

View reviewed changes

merlimat approved these changes Oct 28, 2019

View reviewed changes

Merge branch 'master' into add_batching_bytes

e55c99e

sijie added 5 commits November 12, 2019 16:23

Merge remote-tracking branch 'apache/master' into add_batching_bytes

fe32f97

Fix tests

06d7690

Merge remote-tracking branch 'origin/add_batching_bytes' into add_bat…

17658c0

…ching_bytes

Merge remote-tracking branch 'apache/master' into add_batching_bytes

37051b7

Fix PerformanceProducer

7a2b514

Merge remote-tracking branch 'apache/master' into add_batching_bytes

4a6f3b6

codelipenghui merged commit dedfb2b into apache:master Nov 15, 2019

jiazhai mentioned this pull request Nov 25, 2019

Use message bytes instead of message number for producer pending messages #5739

Closed

sijie mentioned this pull request Dec 27, 2019

ISSUE-5739: Use message bytes instead of message number for producer pending messages streamnative/pulsar-archived#187

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[client] Introduce `batchingMaxBytes` setting in pulsar producer #5045

[client] Introduce `batchingMaxBytes` setting in pulsar producer #5045

sijie commented Aug 26, 2019

merlimat Aug 26, 2019

sijie Aug 26, 2019

merlimat Aug 26, 2019

sijie Aug 26, 2019

sijie commented Aug 26, 2019

codelipenghui Aug 27, 2019

codelipenghui Aug 27, 2019

sijie Aug 27, 2019

codelipenghui Aug 27, 2019

codelipenghui Aug 27, 2019

sijie Aug 27, 2019

merlimat Aug 27, 2019

codelipenghui commented Sep 5, 2019

jiazhai commented Sep 9, 2019

aahmed-se commented Sep 9, 2019

sijie commented Sep 9, 2019

sijie commented Sep 16, 2019

sijie commented Sep 16, 2019

codelipenghui left a comment

codelipenghui commented Sep 17, 2019

sijie commented Sep 19, 2019

sijie commented Sep 22, 2019

jiazhai commented Sep 28, 2019

jiazhai commented Oct 7, 2019

jiazhai commented Oct 15, 2019

merlimat left a comment

codelipenghui commented Nov 10, 2019

codelipenghui commented Nov 11, 2019

sijie commented Nov 12, 2019

	* Set the maximum number of bytes permitted in a batch. <i>default: 1MB</i>
	* Set the maximum number of bytes permitted in a batch. <i>default: 128KB</i>

[client] Introduce batchingMaxBytes setting in pulsar producer #5045

[client] Introduce batchingMaxBytes setting in pulsar producer #5045

Conversation

sijie commented Aug 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sijie commented Aug 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codelipenghui commented Sep 5, 2019

jiazhai commented Sep 9, 2019

aahmed-se commented Sep 9, 2019

sijie commented Sep 9, 2019

sijie commented Sep 16, 2019

sijie commented Sep 16, 2019

codelipenghui left a comment

Choose a reason for hiding this comment

codelipenghui commented Sep 17, 2019

sijie commented Sep 19, 2019

sijie commented Sep 22, 2019

jiazhai commented Sep 28, 2019

jiazhai commented Oct 7, 2019

jiazhai commented Oct 15, 2019

merlimat left a comment

Choose a reason for hiding this comment

codelipenghui commented Nov 10, 2019

codelipenghui commented Nov 11, 2019

sijie commented Nov 12, 2019

[client] Introduce `batchingMaxBytes` setting in pulsar producer #5045

[client] Introduce `batchingMaxBytes` setting in pulsar producer #5045