Add Redis Stream streaming implementation
This was migrated from the 3.x version I'm using and it should work.
Microsoft Reviewers: Open in CodeFlow
Many thanks for this PR, @buzzers. We will take a closer look at it tomorrow. Thank you for your patience.
Hi @buzzers ,
It seems that your implementation doesn't guarantee stream event order. While it is true that some providers don't guarantee it (like AzureQueue), it should be the exception (and we will probably separate them from truly rewindable streams in the future).
It appears that the Redis streaming API allow querying from ranges, which would allow us to have a proper rewindable Redis implementation, that would have the same behavior than EventHub.
Do you think you could switch to the range API instead? It would be tremendously valuable to have that in Orleans.
@benjaminpetit
Of course, rewindable streams are very valuable. I will refer to EventHub in the near future and try to reimplement it.
But here's a question I'm not sure yet:
Does the rewindable stream provide an interface to manually clean up messages that are no longer needed?
This is important because Redis streams does not provide automatic cleanup of expired messages.
And because the messages of Redis streams all exist in memory, if the rewindable stream cannot manually clean up expired messages, there is a high probability that it will cause Redis OOM.
For EventHub it is not needed since messages are purged after n days.
But message deletion, if needed, should be done in IQueueAdapterReceiver.MessagesDeliveredAsync() (it seems that's what you did in this PR)
@benjaminpetit
IQueueAdapterReceiver.MessagesDeliveredAsync() can indeed be used to clean up expired messages. But this might not be very elegant, as Redis commands will be called more often than necessary.
But if there is no other better way, there is no problem with cleaning here.
Is work on this abandoned? I am really looking forward for this to be merged.... @ReubenBond @benjaminpetit feels like this need some reviewing from you guys