merlimat commented on issue #4062: Delayed message delivery implementation
URL: https://github.com/apache/pulsar/pull/4062#issuecomment-484240339
 
 
   > DelayedDeliveryTracker and DelayedMessageIndex are just two different 
implementations of one same things. If the current implementation of 
DelayedDeliveryTracker is acceptable, why the proposal of a time-partitioned 
DelayedMessageIndex is not acceptable? People can choose which implementation 
to use by configuring a configuration in the broker configuration.
   
   That's a very good point. It would be good to have a DelayedDeliveryTracker 
as an interface and we can have different implementations. 
   
   That will help: 
    1. Accommodate different scenarios
    2. Easily experiment with different implementation approaches 
   
   I'll update this PR to make the interface configurable.
   
   > Lastly, PIP-26 already presents changes regarding api, protocol, namespace 
policies and many other changes around this area. Shall we just pickup the 
proposed changes there instead of starting a new effort?
   
   In PIP-26 the proposed API methods were:
   
   ```java
   // message to be delivered at the configured delay interval
   producer.newMessage().delayAt(3L, TimeUnit.MINUTE).value("Hello 
Pulsar!").send();
   
   // message to be delivered at the configure time.
   producer.newMessage().scheduleAt(new Date(2018, 10, 31, 23, 00, 00))
   ```
   
   In this PR I'm proposing: 
   
   ```java
   producer.newMessage().deliverAfter(3, TimeUnit.MINUTE).value("hello").send();
   
   producer.newMessage().deliverAt(timestamp).value("hello").send();
   ```
   
   My reasons are:
    * `delayAt()` seems confusing because "at" in all timing APIs is used for 
absolute positioning
    * I'd rather keep the same prefix `deliverAt()` / `deliverAfter` to make it 
visually clear these are 2 alternative ways to configure the same feature.
    * Date vs timestamp. I have no strong opinion there since both are 
basically interchangeable (eg: `new Date(timestamp)` and `date.getTime()`). I 
was using a timestamp since that is what we're already using for publishTime 
and eventTime.
   
   For the protobuf metadata change, in PIP-26 was :
   
   ```protobuf
   // the message will be delayed at delivery by `delayed_ms` milliseconds.
       optional int64 delayed_ms        = 18;
   ```
   
   Though that won't support specifying absolute time of scheduling scheduling.
   
   Instead, I propose to start with 
   ```protobuf
   // Mark the message to be delivered at or after the specified timestamp
   optional uint64 deliver_at_time = 18;
   ```
   
   Initially, with relative delays the client will just apply based on its 
current time. Once we have broker assigned timestamp (stored within the message 
metadata), then we could add a second field. 
   Alternatively, we could as well start with 2 fields (abs and relative) and 
have the broker do the math based on producer assigned publish time.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to