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 2018/12/30 22:54:29 UTC

[GitHub] lovelle commented on issue #3267: Support set publish time on broker side

lovelle commented on issue #3267: Support set publish time on broker side
URL: https://github.com/apache/pulsar/pull/3267#issuecomment-450592375
 
 
   Thanks for your comments, I answer you each point:
   
   1. I agree that is not acceptable to deserialize and serialize the entire payload just to change publish time (or any field). But, AFAIK, on the added method [`modifyMessageMetadata()`](https://github.com/lovelle/pulsar/blob/feature/set_publish_time_on_broker/pulsar-common/src/main/java/org/apache/pulsar/common/api/Commands.java#L265) the entire payload is just read until message metadata (which has a maximum possible size of 1Kb) and write the uint64 to the buffer (new epoch) and update the new checksum if present, I don't think this is a considerable overhead, please correct me if I'm wrong.
   I agree this behavior should be allowed to be turned on/off by client.
   
   2. Yep, nice catch up. Therefore, only the greater epoch would be the one present on metadata, isn't it?
   
   3. I think the answer to this might be 'it depends'. It depends in the way you think about the 'delayed messages' feature
   
       I think adding a new field is a good solution, the only issue I can see here is that the consumer is related with the decision made by the producer of setting this new field, and the main goal of #3155 is to make delay on consumer messages regardless of the producer.
   Anyway, we can add a new optional field to set this 'publish time' set by default by the client, maybe with the ability to be modified by the user if they want? Another downside of this would be the producer must update pulsar client too and the consumer should be aware that the producer has been updated with new version.
   
   4. Yep, but depends on the meaning given to this specific field, either the current publish time or a new one, it depends if it means the epoch since the message has been created or the epoch since the user has created the message, for the former one we need to do what you said (do we have something that identifies the message as a published by replication?), but if it is the latter case I think we don't need to do anything more.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services