haskell-distributed / distributed-process

Cloud Haskell core libraries
http://haskell-distributed.github.io
711 stars 96 forks source link

Expose identifier for a connection bundle #406

Open avieth opened 7 years ago

avieth commented 7 years ago
-- Identifies a "heavyweight" connection which carries 0 or more "lightweight"
-- connections.
type ConnectionBundle = Word32

data ErrorEvent =
 ...
    -- EventConnectionLost indicates which bundle was lost
  | EventConnectionLost EndPointAddress ConnectionBundle
  ...

data Connection = Connection {
    ...
    -- Each connection knows which bundle carries it.
  , bundle :: ConnectionBundle
  }

Why?

When a connection is lost, an EventConnectionLost peer must be posted and ultimately come out of receive at that end point before any ConnectionOpened events to the same peer. While that event sits in the queue, new connections may be established to that same peer. Once the EventConnectionLost peer comes out of the queue it's impossible to determine which of the existing outbound lightweight connections are affected by it. By including a ConnectionBundle on every Connection and on the EventConnectionLost, it's possible to determine whether an outbound connection has been severed without trying to send on it.

qnikst commented 7 years ago

Looks reasonable to me.

facundominguez commented 7 years ago

Another option is to get rid of the connection bundle concept and report connections going down individually. This would need some careful thought, as I don't think the need for bundles is well documented.

avieth commented 7 years ago

Another option is to get rid of the connection bundle concept and report connections going down individually. This would need some careful thought, as I don't think the need for bundles is well documented.

This would be nice too, and it's easy to implement in network-transport-tcp (I've done it). Whenever ConnectionLost for a certain peer is given, every connection to that peer will have been closed. Same for EndPointClosed/TransportClosed events: all connections will have been closed before these events come out.

But this isn't enough for the use case I have. It involves connecting A to B, and expecting B to connect to A so as to give a bidirectional connection. If A connects successfully to B and sends data, but then the connection is lost before B connects to A, A cannot determine that its connection has gone down unless it tries to send more data. In this case we would want some time out anyway, but if the connection is lost then we shouldn't have to wait for the time out. By giving the connection bundle, the thread which receives the events can inform the thread which is waiting for the connection from B that it won't come.

Another option would be to have something like

withConnection :: EndPoint -> (Connection -> IO t) -> IO t

and throw an asynchronous exception to the Connection -> IO t whenever that connection is broken.

facundominguez commented 7 years ago

I was thinking more along the lines of modifying the interface as:

data ErrorEvent =
 ...
    -- EventConnectionLost indicates which connections were lost
  | EventConnectionLost [Connection]

or using a ConnectionId for outgoing connections instead of the actual Connections.

However, IIUC, bundles are necessary to ensure at the d-p level that the receiving side notes the failure instead of delivering new messages to processes as if no failure had occurred. Otherwise messages could be dropped or arrive disordered. Would be quite a refactoring to continue ensuring that.