kernelci / kernelci-api

KernelCI API - Database - Pub/Sub
GNU Lesser General Public License v2.1
9 stars 17 forks source link

Add support for single consumers of pub/sub events #338

Open gctucker opened 1 year ago

gctucker commented 1 year ago

Currently, all the events are received by all the subscribers when they match the subscription criteria. In some cases, it makes more sense for the event to only be received by one subscriber, for example to distribute jobs across a pool of services.

JenySadadia commented 1 year ago

As per my investigation, Redis Pub/Sub does not provide any way to distribute messages over a group of subscribers. Redis List or Stream can be helpful in this case. Redis list is a preferable option here if we take into account the complexity of streams. Investigating it further.

gctucker commented 1 year ago

When publishing a message to a redis queue, only one subscriber will get it. The current implementation creates a queue for each subscriber, so all the messages are sent to all the subscribers. The feature we're missing is to use a single queue with multiple subscribers so that only one of the group of subscribers receives each message.

gctucker commented 1 year ago

Looking at the Redis commands I see there are different ways to dispatch messages. The approach we're using now relies on PUBLISH and SUBSCRIBE, where every message sent with PUBLISH on a channel is received by all the subscribers. This is basically the standard pub/sub approach and the publisher doesn't need to know about subscribers (basically a "broadcast" system).

Another workflow is to use RPUSH for sending messages to a queue and BLPOP to wait for a message on the receiving end. In this scenario, if several clients are running BLPOP only one will receive the message. That works well for load balancing where we want only one client to deal with the event (for example, scheduler services).

With these features, I think we can address all the use-cases with standard Redis functionality. How does that sound?

gctucker commented 1 year ago

When publishing a message to a redis queue, only one subscriber will get it. The current implementation creates a queue for each subscriber, so all the messages are sent to all the subscribers. The feature we're missing is to use a single queue with multiple subscribers so that only one of the group of subscribers receives each message.

Edit: This wasn't really accurate. As explained in the last comment, lists actually work like this (one recipient) but channels despatch messages to all the subscribers.

JenySadadia commented 1 year ago

Looking at the Redis commands I see there are different ways to dispatch messages. The approach we're using now relies on PUBLISH and SUBSCRIBE, where every message sent with PUBLISH on a channel is received by all the subscribers. This is basically the standard pub/sub approach and the publisher doesn't need to know about subscribers (basically a "broadcast" system).

Another workflow is to use RPUSH for sending messages to a queue and BLPOP to wait for a message on the receiving end. In this scenario, if several clients are running BLPOP only one will receive the message. That works well for load balancing where we want only one client to deal with the event (for example, scheduler services).

With these features, I think we can address all the use-cases with standard Redis functionality. How does that sound?

Yes, Redis List can be used here. We can have a publisher that notifies all the subscribers when the queue is updated. Only one consumer will be able to read the message. Also, I was looking into Redis Streams. It has implemented a consumer group mechanism called XGROUP. The stream distributes messages among the consumers from a group automatically for load balancing. But for that, we need to completely rework the current Pub/Sub implementation in API.

gctucker commented 1 year ago

But for that, we need to completely rework the current Pub/Sub implementation in API.

Yes, and I think this is fine. We can have a small experiment first to see how this would work. The API endpoints for regular pub/sub with broadcast messages shouldn't need to change from a client point of view, and even if that needed to change we could also do that and now is a good time for this kind of changes before we reach the production-ready stage.

JenySadadia commented 1 year ago

But for that, we need to completely rework the current Pub/Sub implementation in API.

Yes, and I think this is fine. We can have a small experiment first to see how this would work. The API endpoints for regular pub/sub with broadcast messages shouldn't need to change from a client point of view, and even if that needed to change we could also do that and now is a good time for this kind of changes before we reach the production-ready stage.

Makes sense. Yes, we can have a PoC first.

gctucker commented 1 year ago

Redis Streams: https://redis.io/docs/data-types/streams/ Redis Lists: https://redis.io/docs/data-types/lists/

As discussed on Slack, it looks like streams are more for monitoring live events in real-time (e.g. progress of a job on a web dashboard in our case, we we could expose via a websocket) whereas lists are more about inter-processor communication for synchronising work which is what we need for load-balancing. So I believe Redis lists provide what we need for this particular issue.

JenySadadia commented 1 year ago

I have done an initial investigation on this issue with Redis List.

Below is the implementation details of the integration:

Tested the integration with kci pubsub commands. It works well in terms of load balancing for multiple subscribers with the same subscription ID.

I am working on further testing with pipeline. I'll send a PR with PoC soon.

gctucker commented 1 year ago

Great, thanks for the initial investigation.

Current PubSub mechanism is as-is to keep track of subscriptions and channels. We'll use redis list to push and pop messages instead of broadcasting messages with redis.pubsub whenever new publishers and subscribers are added

I'm not sure what you mean here, we need to support both use-cases so probably different endpoints or at least arguments to subscribe either to broadcast messages or unicast within a pool of subscribers. I believe a separate pair of endpoints would be clearer.

JenySadadia commented 1 year ago

Please see https://github.com/kernelci/kernelci-api/pull/365

JenySadadia commented 1 year ago

Great, thanks for the initial investigation.

Current PubSub mechanism is as-is to keep track of subscriptions and channels. We'll use redis list to push and pop messages instead of broadcasting messages with redis.pubsub whenever new publishers and subscribers are added

I'm not sure what you mean here, we need to support both use-cases so probably different endpoints or at least arguments to subscribe either to broadcast messages or unicast within a pool of subscribers. I believe a separate pair of endpoints would be clearer.

Yes, we would need to have separate endpoints for broadcasting and unicasting. What I meant was for this use case we'll have the same mechanism for storing subscriptions against channels that we currently have.