Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specification: nix group_expiry setting (or at least, 1-day default) #1371

Open
adamhooper opened this issue Oct 28, 2019 · 0 comments
Open

Comments

@adamhooper
Copy link

I maintain https://github.com/CJWorkbench/channels_rabbitmq.

I propose nixing group_expiry from the Channel Layer specification. It should be an option specific to the Redis layer.

Redis has a global store of group memberships. When Daphne dies, the membership info remains in a Redis data structure -- it has "leaked." channels_redis can't delete leaked group memberships (by definition); so it deletes expired group memberships instead, as a proxy. From what I can tell, "expiry" was invented to handle "leaked memberships." By default, a membership "expires" one day after creation, according to the spec.

I also surmise from some Googling that group_expiry can sometimes defend the Redis layer against full buffers -- that's django/channels_redis#384, and group_expiry doesn't solve it in general.

channels_rabbitmq doesn't have "leaked memberships." Memberships are subscriptions, and they're stored as part of the Daphne-RabbitMQ ASGI connection. If Daphne dies, RabbitMQ cleans up the subscriptions.

And channels_rabbitmq defends against full buffers more sensibly: it gives each message a TTL, and it warns when dropping old messages. If the user forgets to group_discard(), the warning highlights the problem, and old messages don't pollute the buffer so a production server stays up.

The upshot: with channels_rabbitmq, group_expiry does no good. It only solves problems inherent to the Redis layer.

group_expiry certainly does harm. It stalls Websocket connections that are older than a day -- by default.

With the Redis layer, the group_expiry feature and default do more good than harm. With the Channels layer, the group_expiry does no good -- only harm.

Please remove group_expiry from the Channel Layer Specification. It belongs in the Redis layer only.

adamhooper added a commit to CJWorkbench/channels_rabbitmq that referenced this issue Dec 30, 2019
See django/channels#1371 for rationale.

Also, document "normal channels" (i.e., job queues) and what users
should do if they want job queues.

[closes #18]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant