You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "dao-jun (via GitHub)" <gi...@apache.org> on 2024/04/05 13:17:56 UTC

[PR] [improve]Improve batching message doc [pulsar-site]

dao-jun opened a new pull request, #878:
URL: https://github.com/apache/pulsar-site/pull/878

   Improve the batching message doc
   
   ### ✅ Contribution Checklist
   
   <!--
   Feel free to remove the checklist if it does not apply to your PR
   -->
   
   - [x] I read the [contribution guide](https://pulsar.apache.org/contribute/document-contribution/)
   - [x] I updated the [versioned docs](https://pulsar.apache.org/contribute/document-contribution/#update-versioned-docs)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [improve]Improve batching message doc [pulsar-site]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #878:
URL: https://github.com/apache/pulsar-site/pull/878#issuecomment-2039990577

   > Fixes: https://github.com/apache/pulsar/issues/22439
   
   I don't think that the documentation alone is a sufficient resolution. The Javadoc of the Pulsar Java client needs to be updated too.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [improve]Improve batching message doc [pulsar-site]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #878:
URL: https://github.com/apache/pulsar-site/pull/878#discussion_r1553787770


##########
docs/concepts-messaging.md:
##########
@@ -427,6 +427,13 @@ Consumer<byte[]> consumer = pulsarClient.newConsumer()
         .subscribe();
 ```
 
+:::note
+
+Send messages by synchronous API `send` will disable batching, and the message will be sent individually.
+It is for the purpose of reducing the latency of sending messages and preventing blocking of the producer thread.

Review Comment:
   This is not accurate. "producer thread" is vague in this case. is it an internal thread or what thread is it referring to? I guess a well known concept is "caller's thread" or "calling thread". However, the explanation would have be be better.
   
   One explanation is simply that before the send message returns, the batch would have to be sent and the broker would have to return a message id for the sent message. In most usecases, no more messages could be added to the same batch since the caller thread is blocked so the decision has been made to simply trigger immediate sending of the message when the synchronous API is used. 
   I'm not exactly sure how to put this in the docs.
   



##########
docs/concepts-messaging.md:
##########
@@ -427,6 +427,13 @@ Consumer<byte[]> consumer = pulsarClient.newConsumer()
         .subscribe();
 ```
 
+:::note
+
+Send messages by synchronous API `send` will disable batching, and the message will be sent individually.

Review Comment:
   This comment isn't accurate. The batching isn't disabled. The current batch is triggered immediately after sending the message. https://github.com/apache/pulsar/blob/ffff639a1b73a34bbb5115503d4c7783bb2a2770/pulsar-client/src/main/java/org/apache/pulsar/client/impl/TypedMessageBuilderImpl.java#L82-L86
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [improve]Improve batching message doc [pulsar-site]

Posted by "dao-jun (via GitHub)" <gi...@apache.org>.
dao-jun commented on PR #878:
URL: https://github.com/apache/pulsar-site/pull/878#issuecomment-2039785232

   ![image](https://github.com/apache/pulsar-site/assets/21362791/55c53e9b-966e-4efd-bac4-8e99e00ca170)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [improve]Improve batching message doc [pulsar-site]

Posted by "dao-jun (via GitHub)" <gi...@apache.org>.
dao-jun commented on code in PR #878:
URL: https://github.com/apache/pulsar-site/pull/878#discussion_r1573879880


##########
docs/concepts-messaging.md:
##########
@@ -427,6 +427,13 @@ Consumer<byte[]> consumer = pulsarClient.newConsumer()
         .subscribe();
 ```
 
+:::note
+
+Send messages by synchronous API `send` will disable batching, and the message will be sent individually.

Review Comment:
   Yes I know it, it's just that my statement is not accurate.
   
   How about change it to: 
   ```txt
   Send messages by synchronous API `send` will trigger the batch to be sent immediately, even the batch is not full.
   It is for the purpose of reducing the latency of sending messages and preventing blocking of the caller's thread.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org