Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep same connection ID when reconnecting #339

Open
alecgibson opened this issue Jan 16, 2020 · 3 comments
Open

Keep same connection ID when reconnecting #339

alecgibson opened this issue Jan 16, 2020 · 3 comments

Comments

@alecgibson
Copy link
Collaborator

We spoke about allowing Connections to keep their existing ID when reconnecting so we can have a persistent src across all ops belonging to an interruptible "session" on a flaky connection.

We'd suggested allowing the Connection to optionally propose an ID to the Agent (rather than just always being assigned one). However, if I understand this all correctly, then the connection flow (as it stands) looks something like:

  • Open a socket between client and server boxes
  • In parallel:
    • Construct a client-side Connection object with this socket
    • Construct a server-side Agent object with this socket
  • Connection sets it state to 'connecting' until it receives a message from the Agent
  • Agent sends an 'init' message, with the client ID
  • Connection receives the message, sets its ID and sets its state to 'connected'

If the above is correct (please correct me if I'm wrong), then the only approach I can think of which would allow the Connection to propose an ID — without breaking existing behaviour — would be: propose the ID on creation of the socket, outside of ShareDB (eg as a query string on a websocket URL). This feels like it puts too much onus onto the library consumer, so probably not an acceptable solution?

Other approaches would be breaking changes to the connection handshake:

  • Make the Agent wait for a message from the Connection before sending 'init': breaks when old clients connect to new servers, because the Agent is waiting for a message it will never receive
  • Have the Connection counter-propose its assigned ID: breaks when new clients connect to old servers, because the Agent doesn't know what to do with the new message, and the Connection will wait forever for a response

Of course, we could hide this behaviour behind a feature flag if we want to stay on v1, but any changes to the handshake feel like they really belong to this bigger package of work?

Or perhaps the counter-proposal solution could work by checking the Agent's protocol version? If it's 1, then we know we can't counter-propose, but if it's 2, then we can? (Or some other semver thing like 1.1.0)

@alecgibson
Copy link
Collaborator Author

Actually, any current changes to the protocol will be breaking changes, because Connection currently has a hard-coded check for protocol === 1, so any old clients connecting to a new server with an updated protocol will break, unless we also shim the protocol, so that the Agent's 'init' message looks like:

{
    a: 'init',
    protocol: 1,
    semverProtocol: '1.1.0',
    id: this.clientId,
    type: types.defaultType.uri
  }

That way newer clients would know to check for the semverProtocol, and older clients continue to work as usual. When we bump semverProtocol to 2.x, then we could finally drop protocol, and all existing (very old) clients will (correctly) throw an error when trying to connect to the new server with the breaking protocol.

@alecgibson
Copy link
Collaborator Author

Also, if the above connection flow is correct, then am I right in thinking there's a possible bug where:

  • We create a socket
  • The client faffs around a bit after creating the socket
  • The server-side Agent binds to the socket, and — on construction — sends an 'init' message
  • The client finally constructs a Connection object, but too late, and misses the 'init' message
  • The connection is never established

Given that there's no initial prompt from the Connection that it's ready to be initialised, then at the moment, the fact that it's usually ready is sort of "accidental" and reliant on the fact that you'll probably construct your Connection immediately, and that the socket latency is sufficiently high compared to constructing a Connection?

@ericyhwang
Copy link
Contributor

Noting here that Alec wrote a test to show the above case, where an issue occurs with a noticeable delay between the client creating the socket and it creating a connection:
91641de

All the examples and documentation show socket creation -> connection creation being synchronous, so it's not actually an issue today. But it does complicate things if we want to have the client propose a connection id, as Alec notes further above.

We then discussed a new init sequence that should work in a backwards-compatible way:

  • Client creates a socket
  • Server gets the socket, sends down an "init" message as today, with an extra field that indicates it's a legacy init message.
  • New clients see that extra field and ignore that legacy init message. Old clients don't know about the extra field and initialize as normal.
  • New client sends a new "init" message to the server when connection is created
  • Server sees the new client-sourced "init" message and sends back a new-style "init", which the new client will know how to handle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants