njouanin / hbmqtt

MQTT client/broker using Python asynchronous I/O
MIT License
795 stars 188 forks source link

Replay and retained message logic need work (biggest issue: inactive client sessions grow RAM forever) #237

Open shipmints opened 3 years ago

shipmints commented 3 years ago

Session Message Replay Issues

This is a spot that is broken and currently (and correctly) disabled for qos 0 messages. See https://www.hivemq.com/blog/mqtt-essentials-part-7-persistent-session-queuing-messages/ for persistent session discussion where only QOS 1/2 should be retained for session-reconnect replay.

Here when a client has an active connection to the broker, it will be sent the pending message:

https://github.com/beerfactory/hbmqtt/blob/07c4c70f061003f208ad7e510b6e8fce4d7b3a6c/hbmqtt/broker.py#L709

When an old client session is recorded as having disconnected, every single message is enqueued waiting for the client to reconnect. If the client never reconnects, memory is consumed forever.

https://github.com/beerfactory/hbmqtt/blob/07c4c70f061003f208ad7e510b6e8fce4d7b3a6c/hbmqtt/broker.py#L719-L726

Retained Topic Message Issues

Persistent topic subscription restoration looks like it needs work to ensure retained messages are provided when a non-clean session is restored the list of session topics is not reprocessed through the retained topic replay logic as it does when a new subscription is made.

Client session retrieved from cache but does not correctly reestablish topic subscriptions and will miss retained messages for those topics upon reconnection:

https://github.com/beerfactory/hbmqtt/blob/07c4c70f061003f208ad7e510b6e8fce4d7b3a6c/hbmqtt/broker.py#L381-L387

Here is where the logic for replaying the retained topic cache for new subscriptions is that should be reused for sessions pulled from the session cache:

https://github.com/beerfactory/hbmqtt/blob/07c4c70f061003f208ad7e510b6e8fce4d7b3a6c/hbmqtt/broker.py#L473-L480

Configurable caps could be available to control the broker's behavior for the topic retained messages similar to those proposed for the session message cache, above.