Why not use Redis Pub/Sub?

[acu192]

I began studying RedisChannelLayer because I was trying to fix this issue (related, this issue). I was surprised by the complexity of RedisChannelLayer. I expected it would use Redis Pub/Sub, but it does not.

We went ahead and implemented our own Channels Layer that does use Redis Pub/Sub. It has been in production for a few days, no issues yet. In fact, it's going better than we could have hoped! For one, it fixes that bug (linked above) but it also has decreased the CPU load on our application server and on our Redis server. Woot.

We'd love to contribute it to this project. So, a few questions for the maintainers: First, is that of interest? If so, should we completely replace the current RedisChannelLayer? Or should we add our implementation as an alternative, perhaps naming it RedisPubSubChannelLayer to distinguish the two implementations?

Let me know. 👍

[carltongibson]

Hi @acu192 — I'm certainly interested in looking at this!

Using Redis Pub/Sub is well worth exploring — it's essentially what we're doing, especially with the group send, which has been awkward for lots of folks.

(I think historically the implementation was no more than a reference implementation of the channel layer API...)

Most sensible is to propose a second layer, at least to begin discussion.

Thanks! (Quite exciting. 😀)

[carltongibson]

@qeternity did you see this?

[qeternity]

@carltongibson I did - I replied in the other issue about the bug, but definitely interested.

[aryaniyaps]

I am eager to have this feature, pub sub would mean better scalability and faster group sending.
(edit) should this issue move to a discussion?

[carltongibson]

I don't think we need an extra discussion — there's only us, and we all seem keen. 😀

@acu192 Are you happy to open a PR as the next stage?

[qeternity]

Echoing @carltongibson - would be great to get a PR. We've taken a slightly less ambitious approach which you can see here: #246

But a proper pubsub with a rewrite that has been born out of battle test production conditions is far more preferable.

[acu192]

Thank you all for the feedback. I’ll send in a PR today!

[aryaniyaps]

I have added a comment regarding adding channels to groups from the outside. please check that out.
#247 (comment)

[LiteWait]

@carltongibson @acu192 Was this new channel layer fixing a bug in JUST "group" channels? We don't use them but are seeing similar errors.

[carltongibson]

Hi @LiteWait — can you give the PubSub backend from django/channels#251 a run and see if that resolves your issue? (The example you give in #313 is making lots of reconnections so it seems to be in the same ballpark...)

[acu192]

@LiteWait I only focused on group_send() so I cannot say if send() has the same issue or not. My code for reproducing the bug (and my observations about the bug) are in issue django/channels#1683 so maybe that can help you study your situation more. On of my observations was that the dropped messages only happened if I had my test clients connect-disconnect-reconnect a lot. If I used stable test clients, I would not see the bug. @carltongibson alluded to that above as well.

[carltongibson]

I've merged in django/channels#251 and will prepare a release in the next week. If you can all give it a run and feedback, that'd be handy.

Thanks @acu192!