channels icon indicating copy to clipboard operation
channels copied to clipboard

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

Open adamhooper opened this issue 4 years ago • 0 comments

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 avatar Oct 28 '19 17:10 adamhooper