Hono: Devices stop receiving commands after re-connect

[sophokles73]

1. A device that has subscribed to receiving commands on the AMQP adapter.
2. For some reason the AMQP connection fails.
3. The devices opens a new connection to another AMQP adapter instance and again subscribes to receive commans.

In some cases the device is not receiving any commands on its new connection.

One such case could be:
When the device considers its initial connection to the adapter to have failed, the adapter still believes that its connection to the device is alive. Thus, it does not close the command receiver link that it had opened on behalf of the device. Depending on what adapter instance the device re-connects to, the following happens:

1. If the device re-connects to the same instance, then the device's attempt to re-subscribe to commands will fail because the adapter still has such a link open for the device which the adapter still considers working. Any commands sent to the device from an application will be routed to the adapter where delivery of the command will fail because the connection to the device is broken.
2. If the device re-connects to another instance, that instance also opens a command receiver link for the device and now two such links exist and any command sent to the device by an application will either be routed to the initial adapter (where it gets released) or to the new adapter (where delivery will succeed).

There are two issues to be addressed FMPOV:
1) Adapters should close a command receiver link they have opened for a device if a command for that device cannot be delivered to the device due to a stale/broken connection. I believe that the adapter can determine this condition when it tries to forward the command to the device.
2) Adapters should have some means to announce to other adapter instances that they have opened a receiver link for a device. This way, other instances of the adapter get a chance to close their (stale) receiver links for the same device.

[dejanb]

When the device considers its initial connection to the adapter to have failed, the adapter still believes that its connection to the device is alive. Thus, it does not close the command receiver link that it had opened on behalf of the device.

I think we should be able to handle these issues on AMQP level with heart beats and idle timeouts. Is there a case where these tools are not enough?

Are there other examples of these issues other than an example described above?

[sophokles73]

I think we should be able to handle these issues on AMQP level with heart beats and idle timeouts. Is there a case where these tools are not enough?

For the AMQP adapter you are right. In MQTT the heartbeat needs to be offered by the client, i.e. the client may also choose not to provide a heart beat at all. We could, of course, change the MQTT adapter's behavior to require the client to use a heart beat of, say, 30s max. We could then update the user guides accordingly and reject any connection requests that do not comply. Is that what you had in mind?
Still, we could end up with up to 30s where a device does not get any commands after re-connect ...

[sophokles73]

The 2nd issue should go away once we have implemented #1272 because then we only have one command receiver link per protocol adapter instance, i.e. the links are no longer scoped to devices anymore. The Device Connection service is then used to determine, which protocol adapter instance a device is connected to.

[dejanb]

Adapters should close a command receiver link they have opened for a device if a command for that device cannot be delivered to the device due to a stale/broken connection.

It makes sense to implement the first item in any case.

As for the second, I agree that device connection service is a way to go.

[calohmn]

The 2nd issue has indeed been solved by #1272.

Concerning the 1st issue:
For the AMQP adapter, a timeout in waiting for a delivery update when sending a command to a device already leads to the ~connection~ (EDIT: the link) to the device to be closed (see fix for #1732). What is missing there is that the corresponding command handler gets unregistered as a consequence. (Command consumer links don't have to get closed here any more, since there are only the adapter instance and tenant specific links used now.) I'll prepare a PR.

[Alfusainey]

... already leads to the connection to the device to be closed (see fix for #1732)

If I understand it correctly, I think the fix for #1732 closes the AMQP link with the device and not the connection. so I think the connection with the device should still be active, right?

[calohmn]

Yes, right, it's the link, not the connection, that gets closed.

[calohmn]

Concerning MQTT:
There currently is a timeout mechanism for waiting for the command acknowledgement from the device (commandAckTimeout, default 100ms), but MQTT connection/subscription and command consumer are kept alive there as of now.

I think the corresponding thing to do here, would be to close the MQTT connection (for lack of not being able to close the subscription from the server-side) and close the command consumer.

The timeout mechanism is only for subscriptions with Qos 1, I think for Qos 0 we can't do anything here, then.

[sophokles73]

@calohmn

Based on this observation I would foremost want to rename the AMQP adapter's configuration variable HONO_AMQP_SEND_MESSAGE_TO_DEVICE_TIMEOUT to HONO_AMQP_COMMAND_ACK_TIMEOUT in order to be consistent in the naming here ...

Regarding the consequences of the device not sending an ack in time:
I do not quite see why we would want to keep the connection to the device open in the AMQP case but not in the MQTT case. The device not acknowledging the command should be considered the same erroneous behavior and thus should have the same consequences, or am I mistaken?
SO, FMPOV we should indeed close the connection to such a device for both AMQP as well as MQTT.

[calohmn]

Based on this observation I would foremost want to rename the AMQP adapter's configuration variable HONO_AMQP_SEND_MESSAGE_TO_DEVICE_TIMEOUT to HONO_AMQP_COMMAND_ACK_TIMEOUT in order to be consistent in the naming here ...

This is also something I thought about yesterday. But I thought adopting the ACK_TIMEOUT notion is not the straightforward thing to do because in the AMQP world the "acknowledgement" term isn't used much.

How about doing it the other way round: marking HONO_MQTT_COMMAND_ACK_TIMEOUT as deprecated and introducing HONO_MQTT_SEND_MESSAGE_TO_DEVICE_TIMEOUT to replace it in the future.
For the direction "adapter to qpid" we would have the SEND_MESSAGE_TIMEOUT property, and for the other side, "adapter to device", we would commonly have SEND_MESSAGE_TO_DEVICE_TIMEOUT. And, as this is somewhat implied by the names, I think we should change the default for SEND_MESSAGE_TO_DEVICE_TIMEOUT to be the SEND_MESSAGE_TIMEOUT property value (or its default, 1s).
WDYT?

Concerning the default for these timeout properties (currently 100ms for MQTT, 1000ms for AMQP), I think there should be a common value. I would rather opt for 1000ms.

[calohmn]

I do not quite see why we would want to keep the connection to the device open in the AMQP case but not in the MQTT case. The device not acknowledging the command should be considered the same erroneous behavior and thus should have the same consequences, or am I mistaken?

I am not quite sure about this.
In general, I think the fact that we have to use the "brute-force" approach in MQTT of closing the connection (for lack of other options), doesn't necessarily mean that we can't choose a more fine-grained approach in AMQP.

But to start out with the general question here: Why would a device not send the disposition update or acknowledge the published command? It could be:

Scenario 1: the device hasn't received the command because of some network/connection issue

Scenario 2: the device has indeed received the command message, but there was either a delay (e.g. caused by a GC, thinking about a gateway Java application for example) or an exception (possibly triggered by the specific command) while processing the command.

Let's imagine scenario 1. Both adapter and device don't know there is an network/connection issue (in the AMQP case the heartbeat-related idle timeout is 16s per default). I think sending the DETACH frame for closing the link wouldn't reach the device in the same way that the TRANSFER frame did. So, closing the connection and then closing the socket would be the better option, right?

Now it would be good to also take scenario 2 into account, where closing the link/connection would rather do more harm than good (there could be a telemetry-sender link on the same connection which would also get closed). Maybe we could, upon reaching the timeout, send an empty frame (ie. a heart beat message) first, to see if there is actually an issue with the connection. Only if this frame isn't acknowledged, we close the connection.
I'm not sure whether this can be done from an implementation point of view (without too much effort). So, directly closing the _connection_ would be the alternative then, yes.

---

And another point: do we want to consider such a disposition-update timeout as a means to detect a stale connection also in other places in Hono, supplementing the heartbeat mechanism (default timeout there 16s)?
Then we could use the same approach for the "protocol-adapter -> amqp message network" connection.

[calohmn]

Scenario 2: the device has indeed received the command message, but there was either a delay (e.g. caused by a GC, thinking about a gateway Java application for example) or an exception (possibly triggered by the specific command) while processing the command.
[...]
Now it would be good to also take scenario 2 into account, where closing the link/connection would rather do more harm than good (there could be a telemetry-sender link on the same connection which would also get closed).

To exemplify this:
Consider the case that commands are sent to the device at a rate of 100msg/s. For the _first_ command message in a series of 100, no disposition update is received in time. All following 99 messages are acknowledged in time.
Closing the AMQP connection after one second because of that first message, all while 99 later messages have already been processed successfully, seems wrong.
Therefore I think we need some extra condition to close the AMQP connection.
I would suggest waiting another sendMessageToDeviceTimeout period after the timeout has occurred, and when after that period has elapsed no disposition update or "flow" has been received on that link overall (since sending the timeout-causing message), then close the connection.

@sophokles73 WDYT?

[sophokles73]

Ok, closing the connection doesn't seem to make sense because we would also close a potentially unaffected telemetry link. Agreed.
For your 100 commands/sec example (which seems to be feasible for protocol gateway scenarios only FMPOV), it also doesn't seem to be reasonable to close the command link, right? So, why do we need/want to close the link/connection in the first place? The hearbeat should provide a sufficiently reliable tool for detecting stale connections, shouldn't it?

[calohmn]

So, why do we need/want to close the link/connection in the first place? The hearbeat should provide a sufficiently reliable tool for detecting stale connections, shouldn't it?

Well, that's a valid question of course. My initial thought in that regard was, that a non-received disposition update could give an extra indication of a stale connection, something we can use to inform the downstream application a good deal earlier than the 16s default idleTimeout, that the device is _probably_ not connected anymore.
But a proper way of implementing such an extra connection check would of course be to also take the lower-level heartbeat messages into account. If we haven't received a disposition update in a given time frame, but on the vertx-proton/proton-j level indeed a heartbeat (ie. empty frame) was received in the meantime, it doesn't make sense to see the connection as having gone stale. That's because in our protocol adapter logic we can't really check whether a heartbeat was received on the proton transport level (without resorting to using reflection to access the needed objects/methods).

So, my above suggestion with taking all kinds of received disposition/flow frames into account would be a "best-effort" approach, _if_ indeed we wanted to be able to inform downstream applications sooner than the idletimeout in this case. But, as explained above, this is a bit half-baked. So, as we wouldn't gain much by going through these hoops anyway, I now think we might indeed better leave the link and connection alive and let the idleTimeout handle a stale connection.

[calohmn]

1) Adapters should close a command receiver link they have opened for a device if a command for that device cannot be delivered to the device due to a stale/broken connection. I believe that the adapter can determine this condition when it tries to forward the command to the device.

Command consumer is getting closed in that case now (#1919). Determining the stale/broken connection is done via the usual mechanisms (AMQP heartbeat, MQTT keep alive).

2) Adapters should have some means to announce to other adapter instances that they have opened a receiver link for a device. This way, other instances of the adapter get a chance to close their (stale) receiver links for the same device.

Solved via #1272.