You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/07/31 01:22:58 UTC

[GitHub] [pulsar] rdhabalia opened a new pull request #7705: [pulsar-client] Remove UUID generation on sending message

rdhabalia opened a new pull request #7705:
URL: https://github.com/apache/pulsar/pull/7705


   ### Motivation
   Pulsar client producer requires uuid while sending chunked messages.  right now, Pulsar-client lib generates UUID for every send message which is expensive and impacts publish performance. Therefore, UUID generation shouldn't be expensive and producer should not generate uuid for non-chunked messages.
   
   ### Modification
   - Avoid uuid generation for non-chunk messages
   - Generate uuid for message using global-producer name and message-sequenceId.
   
   **Note**
   This fix should be cherry-picked to 2.6.1 release as well.


----------------------------------------------------------------
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.

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



[GitHub] [pulsar] rdhabalia removed a comment on pull request #7705: [pulsar-client] Remove UUID generation on sending message

Posted by GitBox <gi...@apache.org>.
rdhabalia removed a comment on pull request #7705:
URL: https://github.com/apache/pulsar/pull/7705#issuecomment-666873726


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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.

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



[GitHub] [pulsar] rdhabalia commented on a change in pull request #7705: [pulsar-client] Remove UUID generation on sending message

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #7705:
URL: https://github.com/apache/pulsar/pull/7705#discussion_r463937795



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
##########
@@ -103,7 +102,15 @@
     private final BatchMessageContainerBase batchMessageContainer;
     private CompletableFuture<MessageId> lastSendFuture = CompletableFuture.completedFuture(null);
 
-    // Globally unique producer name
+    /**
+     * Globally unique producer name generated by server. It should be the same as {@link #producerName} unless user
+     * configures {@link ProducerConfigurationData::setProducerName}.

Review comment:
       I have changed it but I don't think `user_producer_id` + `sequence_id` can create UUID because `user_producer_id` can be reused by other producer later on and `sequence_id` can start with `0` again and it will create a duplicate combination which will not be unique across messages on the topic and that can be issue for message chunking.
   eg:
   Producer P1 gives name `userProducer1` and publish message with sequenceId=`0`. Now that process died and another process created producer with similar name `userProducer1` and started message with sequenceId=`0` in which case UUID will not be unique.
   Therefore, using `serverProducerName` always gives a guarantee for unique producer-id and UUID. Adding new string `serverProducerId` will not create any overhead as well. So, can you please let me know your thought on it.




----------------------------------------------------------------
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.

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



[GitHub] [pulsar] srkukarni merged pull request #7705: [pulsar-client] Remove UUID generation on sending message

Posted by GitBox <gi...@apache.org>.
srkukarni merged pull request #7705:
URL: https://github.com/apache/pulsar/pull/7705


   


----------------------------------------------------------------
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.

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



[GitHub] [pulsar] rdhabalia commented on a change in pull request #7705: [pulsar-client] Remove UUID generation on sending message

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #7705:
URL: https://github.com/apache/pulsar/pull/7705#discussion_r464703088



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
##########
@@ -103,7 +102,15 @@
     private final BatchMessageContainerBase batchMessageContainer;
     private CompletableFuture<MessageId> lastSendFuture = CompletableFuture.completedFuture(null);
 
-    // Globally unique producer name
+    /**
+     * Globally unique producer name generated by server. It should be the same as {@link #producerName} unless user
+     * configures {@link ProducerConfigurationData::setProducerName}.

Review comment:
       @merlimat did you get chance to read my above comment before merging the PR?




----------------------------------------------------------------
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.

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



[GitHub] [pulsar] rdhabalia commented on pull request #7705: [pulsar-client] Remove UUID generation on sending message

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #7705:
URL: https://github.com/apache/pulsar/pull/7705#issuecomment-666873726


   /pulsarbot run-failure-checks


----------------------------------------------------------------
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.

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



[GitHub] [pulsar] merlimat commented on a change in pull request #7705: [pulsar-client] Remove UUID generation on sending message

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #7705:
URL: https://github.com/apache/pulsar/pull/7705#discussion_r463890006



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
##########
@@ -103,7 +102,15 @@
     private final BatchMessageContainerBase batchMessageContainer;
     private CompletableFuture<MessageId> lastSendFuture = CompletableFuture.completedFuture(null);
 
-    // Globally unique producer name
+    /**
+     * Globally unique producer name generated by server. It should be the same as {@link #producerName} unless user
+     * configures {@link ProducerConfigurationData::setProducerName}.

Review comment:
       This will always be the same as `producerName`. If the client overrides it, the server will use that name, but if a producer with same name is already connected, it will instead error it out




----------------------------------------------------------------
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.

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