search

moscajs / mosca

MQTT broker as a module
mosca.io
3.2k stars 509 forks source link

Custom Packet TTL #516

Closed behrad closed 8 years ago

behrad commented 8 years ago

Application may need to set a custom ttl for each message, Mosca currently is using options.ttl.packets I propose adding a packet.ttl to the packet argument of publish method, so that storePacket overwrite options.ttl.packets in case.

I can create a PR for redis-persistence to handle this @mcollina

mcollina commented 8 years ago

how would you set this custom ttl?

behrad commented 8 years ago

something like changing https://github.com/mcollina/mosca/blob/master/lib/persistence/redis.js#L370 to .pexpire(packetKey, packet.ttl || this._packetKeyTTL)

mcollina commented 8 years ago

I mean, given that ttl is not part of MQTT, where would you set it in your application?

ttl is one of the features I put in Mosca that I thought made things way more complex than they should be.

behrad commented 8 years ago

I mean, given that ttl is not part of MQTT, where would you set it in your application?

It unfortunately only works when calling server.publish (adding a new attribute to the packet argument) inside mosca, I haven't found a proposal when publishing from a MQTT client yet.

behrad commented 8 years ago

Hmm, I was thinking to go for a pre-publish hook inside mosca, which app overrides to set the specific message ttl :) this may also sound good, however app may need to decode the packet.payload to recognize what ttl to pass to mosca

keanuyaoo commented 8 years ago 
I want to use it in my project,but I am a bit worried about its performance!
Is there any way to cluster  it?
mcollina commented 8 years ago

@behrad currently Aedes does not support ttl, as I think it's too persistence specific (and increase the complexity of the codebase). I want to reduce the maintenance burden, as I struggle to keep up. Are you relying on ttl?

@keanuyaoo

I want to use it in my project,but I am a bit worried about its performance! Is there any way to cluster it?

What do you mean? How does this relates to ttl?

behrad commented 8 years ago

currently Aedes does not support ttl, as I think it's too persistence specific (and increase the complexity of the codebase). I want to reduce the maintenance burden, as I struggle to keep up. Are you relying on ttl?

I'm completely aware of what you are worrying about, however in this specific feature, for the Redis persistence at least, I believe it is not that complex, and no code level logic for handling ttl is involved, and it actually worth it since in a real world scenario with high message throughput, some housekeeping should purge old messages for inactive clients. The special case, is app support for message expiry, which means some messages shouldn't arrive clients after it means expired.

and finally since it is not a standard MQTT feature, we can support this in only persistence providers that the internal storage natively, easily supports TTL, and redis is this case.

mcollina commented 8 years ago

I don't think this is a feature we should support in this codebase. On aedes-persistence-redis will be easy to support (and you probably can roll your own persistence there as well, it's way easier than here).

behrad commented 8 years ago

So I'll create a PR there, if you are ok to merge there 👍