moquette-io / moquette

Java MQTT lightweight broker
http://moquette-io.github.io/moquette/
Apache License 2.0
2.3k stars 818 forks source link

Implement session expiry cleanup for persisted sessions #739

Closed andsel closed 1 year ago

andsel commented 1 year ago

Release notes

Implement the logic to expire the not clean sessions.

What does it do?

Track all expiration time after a session is closed or disconnects. Uses a delay queue to track the not clean session that expires and using a scheduled task, every second, check for the expired sessions and removes from the both SessionRegistry and also from the persistent store (SessionRepositoy).

Why it's important for the user?

This feature let the user to auto-purge the state of the not clean session after a certain amount of time configured at broker level in moquette.conf file through the persistent_client_expiration property which could have the format: <number><unit> such as 1d for 1 day. Admitted values to unit are s m h d w M y which respectively means seconds, minutes, hours, days, weeks, months and years. If not specified this global expiry defaults to INFINITE_EXPIRY which is ~80 years.

andsel commented 1 year ago

Hi @hylkevds may I ask you if you would like to be my buddy in this effort and provide a review to this functionality, please :pray: ?

hylkevds commented 1 year ago

While working on the subscription tree this weekend I noticed that subscriptions are not cleaned up when a session is cleaned up. I noticed because the interceptor didn't get notified of any unsubscribe events when a (clean) session terminated due to a disconnect.

This may need its own issue, WDYT?

andsel commented 1 year ago

@hylkevds I think this could be a bug but:

interceptor didn't get notified of any unsubscribe events

the interceptor is notified only when an UNSUBSCRIBE message is received. When a clean session is wiped due to a disconnection it doesn't mean that the unsubscribe interceptor is notified, or do you thing it should?

hylkevds commented 1 year ago

I think it should, yes. If an interceptor wants to know about subscribe and unsubscribe events it's most likely to keep track of which topics have subscribers so it know which topics to send data to. For that use-case it is important to get all unsubscribe events, regardless of why one happened.

In our specific use-case there is no simple one-to-one mapping between events and the topics that may be interested in those events. It's an (in some cases extreme) many-to-many mapping. One event may have to be sent to potentially thousands of topics. That's why we need to keep track of exactly which topics actually have subscribers, so we can avoid generating messages for topics that have no subscriptions.

Of course the subscription not being cleaned up and the interceptor not being notified are two distinct, but related, issues.

andsel commented 1 year ago

@hylkevds please open an issue describing for that, describing the behavior. It will addressed in a separate PR.

andsel commented 1 year ago

Hi @hylkevds I've addressed all your latest concerns. Please could you review it?! :-)