dionjansen opened a new pull request #67: URL: https://github.com/apache/pulsar-dotpulsar/pull/67
Implements: #45 , #46 ## General comments * I've added a reference to Microsoft.Bcl.AsyncInterfaces to support the IDisposable interface in net5.0. I only had this issue in VSCode for the test and sample projects. This does not influence the tests or running samples on VSC (on mac). * This is still a draft mostly to get the discussion going. * I would like to add some more unit tests though this is a bit hard without a proper mocking framework, would you be open to introducing something like Moq to allow testing classes in a shallow way? Or do you rather have I follow a different unit test strategy? ## Discussion I think we can split up the discussion in 2 parts. 1. The integration in the rest of the lib: where what is created, started and what is optional, general structure/ logic of the lib 2. The implementation of the tracker: performance concerns, disposing @blankensteiner I think firstly I would like to get some thoughts on the current implementation, I mostly focussed now on setting things up so I don't break existing processes: ### Integration 1. Right now I create a `IMessageAcksTracker` instance in the `PulsarClient`, though when clients do not have the consumer configured to use the tracking, a dummy `InactiveMessageAcksTracker` is passed. This instance is passed to the `ConsumerChannelFactory` and used when a channel is created. Does this setup makes sense to you? 2. The `ConsumerChannel` now expects a `MessageQueue` which wraps the tracker and the `AsyncQueue`. The channel now informs the tracker through this queue when acking (and later nacking which I can add later) and the dequeue method automatically starts tracking messages received. 2. When the tracker is started it runs indefinitely I find it hard to find a good place to start this thread, also since I need the consumer to call `RedeliverUnacknowledgedMessages`. Any ideas on how to improve this pattern? Perhaps a static method like some of the `StateMonitor` methods. 3. I wonder how batched messages fit into all this, does a batch have a single message id? And should I be able to just redeliver that message id to release the batch back to the broker? ### Implementation 1. The `MessageAcksTracker` uses a polling mechanism to re-check for timed out messages (either due to being unacked for too long or the nack delay has been exceeded). Is this (generally) what you were thinking about too? Alternatively I could think of an approach where polling is done through a [Timer](https://docs.microsoft.com/en-us/dotnet/api/system.threading.timer?view=net-5.0). Or we could create individual Tasks for each added message to the tracker but I'm concerned of the overhead created by this. 2. Atm I haven't considered the scenario yet that only a nack delay is configured and not an ack timeout in which case we will not have to track all dequeued messages. ---------------------------------------------------------------- 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: us...@infra.apache.org
