eclipse-hono / hono

Eclipse Hono™ Project
https://eclipse.dev/hono
Eclipse Public License 2.0
449 stars 137 forks source link

Set time-to-live on (dis)connectedTtd event messages #2342

Open calohmn opened 3 years ago

calohmn commented 3 years ago

The event messages sent from a protocol adapter when a device (un)subscribes for command messages currently get no ttl ("time-to-live") property set.

This could mean that these event messages get stored for an unnecessarily long period in the broker, especially if the northbound application isn't interested in these events.

For time-constrained command subscriptions (with "time-til-disconnect"/ttd property), we could set the ttl to the ttd value. For other subscriptions we could use a configurable value (configured globally/on the tenant level).

mbaeuerle commented 3 years ago

I wonder if this is a regression of https://github.com/eclipse/hono/issues/1714

calohmn commented 3 years ago

@mbaeuerle It's not a regression since the "Connection events" of #1714 (sent when a device (dis)connects) are different from the (dis)connectedTtd events mentioned here (sent when a device (un)subscribes for commands).

calohmn commented 3 years ago

Actually, the default ttl for a device and the tenant maxTtl are already getting applied here (if "defaults" are enabled for the adapter).

What is missing is taking the ttd value into account. EDIT: Actually, taking the ttd into account doesn't make sense FMPOV. For AMQP/MQTT, where a separate event is sent, the ttd is 0 or -1 anyway. For HTTP/CoAP the ttd would be > 0, but there is no extra event getting sent here. Applying the time-to-live to the telemetry/event message that has the ttd param doesn't look right. The message could have a meaning and right to exist that goes beyond the fact that it is also the "connectedTtd" notification message.

calohmn commented 3 years ago

@Christian-Schmid @mbaeuerle Apart from the tenant max-ttl already getting set as time-to-live here, do you think it makes sense to have a separate connection-event-ttl device/tenant-level default property? This property could apply to both the "(dis)connectedTtd" events and the "Connection events" (#1714).

Christian-Schmid commented 3 years ago

Mhm good question. I would probably tend for having a connection-event-ttl property. The max-ttl for some of our tenants is in our case sometimes quite high with 24 hours.

The timespan as long as someone is interested in a "connection event" is probably much shorter. I would put this more in the time range of a few minutes maximum.