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

Proposal for new transport fallback behaviour #217

Closed
SimonWoolf opened this issue Jan 25, 2016 · 1 comment
Closed

Proposal for new transport fallback behaviour #217

SimonWoolf opened this issue Jan 25, 2016 · 1 comment
Assignees

Comments

@SimonWoolf
Copy link
Member

From @paddybyers plan in https://github.com/ably/wiki/issues/57:

abandon upgrade: do fallback from ws to http. That will slightly degrade connection times for a small minority of connections, and vastly streamline what happens for the majority;

...

d) make sure we understand how we approach fallback hosts vs fallback transports. Right now, since we start with an http transport which is "guaranteed" to work provided the host is reachable, we have a straightforward decision to try a fallback host if it doesn't connect. Now instead if the ws connection fails, do we do the internet-up check first? or the transport fallback? Or we prioritise the transport fallback unless we've successfully connected previously with ws?

Had a think about the easiest way to implement this. Here's a proposal that inserts a short-circuit at the beginning to connect with websockets with the default host -- which really should work for 99% of people 99% of the time -- and keeps the existing flow (internet-up checks, host fallback flow, transport fallback flow) for the remaining 1.99%.

Proposal

New chooseTransport behaviour:
  • if upgrade transport is available:
    • chooseTransportForHost with websocket transport and first choice of host. (if called back with an err within 2s, skip to chooseHttpTransport)
    • wait two seconds. If after that time is no transport pending or active:
      • chooseHttpTransport
  • else
    • chooseHttpTransport
new setTransportPending(transport) behaviour:
  • if transport is an http transport and there is already a websocket transport pending or activated:
    • disconnect the http transport; return
  • else if transport is a websocket transport and there is already an http transport pending:
    • remove the transport activation event handler from the http transport and disconnect it
    • add activateTransport on('connected') handler for the websocket transport and proceed as normal
  • else if transport is a websocket transport with a mode that isn't 'upgrade' and there is already an http transport already activated:
    • disconnect the websocket transport (too late to make it a proper upgrade); return. (Trust in the existing upgrade mechanism to do a proper upgrade in due course).
  • else existing behaviour

(above proposal uses "websocket transport" for what connectionManager calls an upgradeTransport to avoid confusion with a transport using the 'upgrade' mode)


So the net result is:

  1. If shortcircuit websocket connects in two seconds, that's all that happens.
  2. If shortcircuit websocket connects in t seconds for 2 < t < T (where T = time it takes for normal chooseHttpTransport process to result in an activated transport): http connection attempt(s) will be made, but they will be disconnected and discarded before being activated.
  3. If shortcircuit websocket connects in t > T seconds: that websocket connection will be discarded; the existing upgrade mechanism will upgrade the http connection in due course
  4. If shortcircuit websocket fails immediately: http connection will happen immediately.
  5. If shortcircuit websocket just never connects: http connection will happen two seconds later than it does at the moment.

The 2s limit is a balance: too long means the delay in case 5 becomes unacceptable; too short and cases 2 and 3 will start dominating over case 1. I picked 2s at random, would welcome @paddybyers advice on the best value.

@paddybyers @mattheworiordan thoughts?

@SimonWoolf SimonWoolf self-assigned this Jan 25, 2016
@paddybyers
Copy link
Member

I've moved this discussion ^

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

No branches or pull requests

2 participants