Support client disconnections and reconnections

[marcospb19]

To provide a better collaboration experience, clients should be able to reconnect after temporarily losing connection.

Hosts should also be able to shut a session down.

Disconnections

• Non-host intentional disconnection.
  • Add a message to request a successful disconnection. #27
  • Notify everyone that the peer left.
• Host peer intentional disconnection.
  • Send a disconnection message. Notify peers that the session terminated.

Reconnections

• Non-host loses connection.
  • User tries reconnecting, if success, it catches up.
• Host loses connection.
  • Tries reconnecting, if unsuccessful, session ends, and peers are notified.

Reconnection questions:

• What's the best approach to ensure that most of the work done while disconnected won't be lost?
  • Should the server store edits in a buffer?
  • Should the client store edits in a buffer?
• What's a good reconnection timeout?

[marcospb19]

I'm leaning towards having the clients save edits locally while the server hasn't successfully acknowledged that the edit was received.

Those edits are then sent once the connection is reestablished, so clients could go offline for hours or days and converge when reconnecting.

@noib3 would that be possible?

[noib3]

That's definitely possible. The real question is what happens if the peer manages to reconnect to the session. I think we have mainly 2 ways to go about it:

a) when the peer reconnects, the server asks the host for its current SessionState and relays that to the peer. The peer can then apply the edits that it stored to the corresponding buffers in the SessionState and use the results to entirely replace the local buffers;

b) if the server detects that a peer in the session has disconnected without first sending a disconnection notification, it saves the other peers' edits in memory for some time, e.g. 30 secs. If the peer reconnects before the deadline it will receive the edits that were buffered on the server. If not, the server drops them and sends a PeerLeft message to the remaining peers.

Option a):

• pros: easier to implement;

• cons: we need to add a new variant to the nomad_collab_client::InboundMessage to handle this, all the users of the client crate have to re-implement the same logic, and the convergence of the project relies on the users doing the right thing;

Option b):

• pros: we can keep the current API untouched (apart from the server sending ACKs) and the users don't have to worry about anything since the server handles most of the logic;

• cons: harder to implement, introduces a risk of memory DDoS on the server, and it'll make switching to a P2P architecture harder if we ever choose to;

I think we might be able to get the best of both worlds by going for a hybrid approach.

The simplest way to implement ACKs on the server is with a timer, e.g. the server sends 1 ACK per second to all the peers to notify them that all the edits have been received and they can drop them.

When a peer disconnects, we can make the clients keep all their edits by not sending ACKs from the server. Then, if

• the peer doesn't reconnect in time: the server sends an ACK and a PeerLeft to all the remaining peers. The peers can drop the edits that they had been saving, and the session resumes as usual;

• the peer reconnects: the server sends an ACK and a new message type asking the peers to send all the edits they made since the last ACK was received. The peers send those edits to the server, then drop them. The server relays those edits to the peer that reconnected.

There are probably some kinks to iron out, but I think something like this can work.

[marcospb19]

Let's call the hybrid solution "option C".

IMHO, the B DDoS problem is severe, so I'm left with A and C to consider...

---

Before moving on, I think we should discuss what we expect (as users) from the reconnection feature.

I'd like reconnecting to work after hours or days of disconnection, here are some scenarios:

• When I'm pairing and I go for a lunch break, my laptop might sleep due to inactivity, or I might close the lid and take it with me, but when I'm back, I want to continue pairing in the same session.
• If we're pairing, and we stop by the evening to continue by the morning, I'd like to put my computer to sleep, and seemingly re-join the session without ever closing the buffer or editor.
• When my connectivity goes out for some time, and I don't want to lose my thought process, so I keep writing my code, and I want it all to converge when it's back.
• When the power goes out and I'm on my laptop, the same as above.

So, losing connection wouldn't be considered a permanent disconnection, as peers can reconnect at any time (given that they have host's permission to do so, ofc, this might be an opt-in setting).

I think that's the way to provide the best user experience, I want to converge from almost any scenario, because I'd hate to have my work lost, or having to manually re-write changes I already wrote.

---

Also, C, as a hybrid solution, inherits some drawbacks from A and B:

a. C drawbacks inherited from A: It adds more complexity to the API, and we still rely on clients doing the right thing (storing edits until ACK, correctly sending them all when asked) to achieve convergence.
b. C drawbacks inherited from B: Hard-to-implement recovery process for the server.

---

With all that in mind, I'll compare A vs. C.

In contrast with A, C isn't viable for several hours (or days) of disconnection, all peers would have to store edits all in memory until the disconnected peer reconnects, but they might never do so.

So, C requires a short timeout, A doesn't, 1 point for A. (And that's already a major dealbreaker IMHO)

C adds more bandwidth usage, 2 points for A.

C adds more memory usage to all peers instead of just one, 3 points for A.

A relies on a single reconnecting client to behave as expected, meanwhile, C expects all clients to behave as expected when any peer is experiencing connectivity issues, that's more room for error, 4 points for A.

A's recovery process is simpler to implement on the server side than C's, 5 points for A.

I also think A is easier to implement client-side 🤔, but I'm not sure, there is a trade-off.

---

Here's an annoying corner case that's also a dealbreaker with C, consider these steps:

• There are 3 peers connected, Peer 1 is the host.
• Peer 2 disconnects briefly, and starts trying to reconnect.
• Meanwhile, all 3 peers write several edits.
• Peer 3 disconnects (willingly or by accident) before Peer 2 is able to re-stablish the connection.
• Peer 2 re-stablishes the connection.

Now, Peer 2 is waiting for edits that Peer 3 made, but Peer 3 might never join again to send them.

The host, Peer 1, is at a different state because it was receiving edits from Peer 3 while Peer 2 was out, but Peer 2 won't ever receive them.

So the two remaining peers will never converge, and Peer 2 should probably get kicked out of the session.

In short, reconnection can only succeed if nobody else disconnects or fails to send edits back when requested.

We could add some workarounds to the above, but not without additional complexity.

---

To conclude, IMHO A is way better, and we can mitigate its drawback.

1. We can add a reconnection method to our client lib API, and force users to call that if they want to reconnect.
   a. So we can use the type system to enforce that everything necessary is provided.
   b. We can document the invariants in this method docs, and it should be pretty straight forward to comply.
2. With A, we can guarantee that it's impossible for a malfunctioning (or malicious) reconnecting peer to mess with the state of other peers.
   a. It's impossible to guarantee that a client won't diverge by itself, any local bug could cause it, but we should be sure that it won't cascade to others.

[marcospb19]

And to see if we're on the same page... here's what I assume that A looks like:

• Each peer stores every edit locally.
  • Those can be erased from memory right after they're sent via TCP.
    • We don't need additional ACKs besides TCP's one.
• When a peer disconnects, it keeps all local edits.
  • Server realizes that peer just disconnected, and notifies all peers.
• When the peer manages to re-stablish the TCP connection:
  • They ask the host to send the updated session state, which includes:
    • A new encoded CRDT replica.
    • The new buffer state for all files that are currently open.
  • For each edit stored locally:
    • Apply edit to the new replica.
    • Then, send local edit to the server.
    • Erase it from memory.
• Consider peer successfully reconnected.
  • Server notifies other peers.

[noib3]

I'd like reconnecting to work after hours or days of disconnection

I'd also like to go in this direction, but the current architecture assumes that the host is always online. If the host leaves, the entire session ends. To remove this limitation we'll have to think about how we can make the filesystem converge. Eg: what happens if we both go offline and I edit a buffer that you've concurrently renamed or moved under another directory?

And if we manage to solve that, you still won't have access to LSP-related features if the host is offline, unless we send the entire project to every peer.

a. C drawbacks inherited from A: It adds more complexity to the API, and we still rely on clients doing the right thing (storing edits until ACK, correctly sending them all when asked) to achieve convergence

It doesn't. That logic would be handled internally by the client crate, not surfaced to its users.

b. C drawbacks inherited from B: Hard-to-implement recovery process for the server.

I wouldn't call it "hard", the server just doesn't send ACKs if a peer is reconnecting. Definitely harder than not having to do that though.

C requires a short timeout, A doesn't.

True.

C adds more bandwidth usage, 2 points for A.

The exact opposite is true. In C, if a peer reconnects the other peers just send the edits they made in the meantime. In A the host has to send the entire state of the project.

C adds more memory usage to all peers instead of just one.

True, but I think we'll eventually want all the peers to keep their edits stored locally to implement things like infinite undo and others, so it shouldn't matter.

A relies on a single reconnecting client to behave as expected, meanwhile, C expects all clients to behave as expected when any peer is experiencing connectivity issues

The number of peers is irrelevant. In A you're relying on the user of the client library to do the right thing, while in C you're relying on the library itself. We control the latter, not the former.

A's recovery process is simpler to implement on the server side than C's

True, in fact as far as the server is concerned there's not any "recovery process". If the peer comes back online it just asks the server for the state of the project, just like it would when joining for the first time.

Here's an annoying corner case that's also a dealbreaker with C, consider these steps:

Well, true, but this is at the basis of eventual consistency. In general, you need every peer to come online and send their edits for the system to converge. This is not a bug.

We can add a reconnection method to our client lib API, and force users to call that if they want to reconnect.
a. So we can use the type system to enforce that everything necessary is provided.
b. We can document the invariants in this method docs, and it should be pretty straight forward to comply.

The reconnection itself is done by the library, not by its users. We might send an InboundMessage::Reconnecting message to let the user know that they're offline and we're trying to reconnect. If the peer successfully reconnects we send a InboundMessage::Reconnected { new_state: SessionState }. It's then the user's job to take the new SessionState and combine it with the edits they stored while offline to produce the correct final state.

What I really don't like about A is:

1. it forces the host to send the entire project state instead of a small number of deltas;
2. when you receive the state, you have to replace whole buffers instead of just applying a few insertions and deletions.

Both problems might be solvable in cola by adding:

impl Replica {
    fn diff(&self, version_map: &VersionMap) -> ReplicaDiff { .. }
}

where VersionMap is an internal data structure in cola that keeps track of the received state at each peer, and ReplicaDiff holds the Insertions and Deletions that should be applied at the replica whose state is VersionMap to bring it up to date with self. But I'm not sure it's possible to implement this method with the current architecture of cola. I'll think about it.

A also suffers from the convergence problems I mentioned at the beginning. We'll first need to solve those problems if we decide to go through with A.

[marcospb19]

Good points 😅.

I'd also like to go in this direction, but the current architecture assumes that the host is always online.

Right, 🤔 I think we should try converging the filesystem then.

And if we manage to solve that, you still won't have access to LSP-related features if the host is offline, unless we send the entire project to every peer.

That's fine for me, if I'm disconnected, then I'll write some code that doesn't compile and fix it afterward.

Anyways, we should consider your idea of sending small projects to enable local LSPs.

Well, true, but this is at the basis of eventual consistency. In general, you need every peer to come online and send their edits for the system to converge. This is not a bug.

If we settle on a short timeout then that's unlikely to be a problem, otherwise, reconnecting after hours is likely to fail, which would be frustrating.

What I really don't like about A is:

1. it forces the host to send the entire project state instead of a small number of deltas;
2. when you receive the state, you have to replace whole buffers instead of just applying a few insertions and deletions.

Both problems might be solvable in cola by adding:

impl Replica {
    fn diff(&self, version_map: &VersionMap) -> ReplicaDiff { .. }
}

Replica::diff sounds ideal, but if that's not doable, I'd still take 1 and 2.

(Actually, 1 gets hard to swallow if we decide to send entire projects.)

[marcospb19]

We discussed this yesterday.

For now, we decided to implement short reconnections with simple filesystem convergence.

So we'll be postponing long reconnections due to the effort it requires to do it right (we don't want to ship a half-baked solution with project tree locks).