Should Disconnect and hearbeat be symmetrical ?

[maxired]

Hi,

I am wondering whether we should change the behavior of heartbeat and disconnect to be symmetrical.

Let me explain :

• In the Worker side, after I called 'close', there is currently no way to know that the disconnection has been correctly done.
  • I suggest that upon reception of a W_DISCONNECT message from a Worker, the broker acknowledge this. Maybe we should aslo send a W_DISCONNECT, or maybe introduce a new message such as W_DISCONNECTED.
• On the client side, there is no way to know that a broker is still there. While we send W_HEARTBEAT to the broker, we never get some back. We can also see that 'HEARTBEAT_LIVENESS' is never used on the client.
  • I suggest that upon reception of a HEARTBEAT_LIVENESS without RequestId, the broker answer to the client with a W_HEARTBEAT.
• On the client side, when I call 'stop', we don't contact the broker(s) to tell them that we leave. I am not sure how zeromq will behave in the case where the broker try to forward a reponse from a worker but the client has been disconnect before.
  • Would an exchange of W_DISCONNECT message make sense in this case ?

The goal I am trying to pursue here, is to be able to build a callback system on top of connection/deconnection event, so I never have to setup timeout as a user of the library.

Please let me know your opinion about all of theses.

[prdn]

Hi,

I agree on the Client/Broker heartbeating. My last commit fixes that. Also the client now emits 'connect' (on start or reconnect) and 'disconnect' (on stop or disconnect) events. Not sure if the Broker needs to know if a client is disconnected. I don't think it should care about that and ZeroMQ should discard messages after some time (ZMQ_LINGER socket option).

When a Worker calls 'close' a 'disconnect' message is issued to the Broker. So the Broker knows that the Worker is not alive and the Worker knows that himself is no more connected to the Broker. Am I missing your point here?

[maxired]

Hi,

thanks for you answer and commits. My point was to say that, since we can't be sure that a message has been correctly sended and received, maybe we should send application level acknowledges.

For example, I was thinking that the 'connect' event on the client might be emitted only after the first hearbeat answer received from the broker.

When a Worker calls 'close' a 'disconnect' message is issued to the Broker. So the Broker knows that the Worker is not alive and the Worker knows that himself is no more connected to the Broker.

In fact, in the current implentation, https://github.com/prdn/pigato/blob/44cbd29fd2deeac44ae1089406b144b5ab33e706/lib/Worker.js#L40L43 , the worker is supposed to send a 'disconnect' message, but the next instruction is to close the socket. This means that the message is never issued to the Broker. If we wait for an acknowledge of the W_DISCONNECT, this will not happen.

[prdn]

Hi,

zeromq leave to the developer the logic to handle a disconnection.

One problem with a worker that waits the ack from a broker for its disconnection is that we should not close worker socket until we receive that. Is this something that we want?

The broker implement a requeue solution. When it finds that a disconnected worker has in flight requests it requeues them. That in my opinion is transparent and fault tolerant.

Then I do not want to add too much logic on the client / worker side.

We can move the connection events after the first heartbeat has received. That doesn't prevent to send messages even before that because the linger option takes care of the send queue.

[maxired]

Hi

One problem with a worker that waits the ack from a broker for its disconnection is that we should not close worker socket until we receive that. Is this something that we want?

I don't see any problem with that. We should also probably add a timeout in case the broker doesn't answer.

I otherwise agree with all of your message. Again, in the current implementation, the disconnect message is not sended. The requeue solution works fine but I think it add latency. I'll try to do a unit test to demonstrate that.

[prdn]

Think we have closed with b60ee98ab962bd8d7d5efcdff2e0a5ea8d29262e