josephg / node-browserchannel

An implementation of a google browserchannel server in node.js
287 stars 47 forks source link

Implementing Session Affinity / Sticky Sessions #35

Closed wenzowski closed 10 years ago

wenzowski commented 10 years ago

As previously noted, there are very good reasons to require session affinity when using Browserchannel with multiple nodes. There are many ways to implement this, but I'm attempting to route requests to the correct worker by performing consistent hashing on a single GET variable.

When hashing on OSID, I observe consistent results. Unfortunately, when starting the channel, OSID is only set when reconnecting so using this token results in one extra channel negotiation on every page load.

Since the old SID is being sent as OSID on every renegotiation to prevent the ghosting problem, the server deletes the session referenced by OSID when the key is set. That doesn't seem to stop the client though: once a sessionId has been replaced the client appears to send the the oldSessionId until another oldSessionId replaces it.

If the server were to test SID !== OSID before attempting to delete the session, then once the client receives the state=ok and its session id, could it set the oldSession early, allowing the next /bind request to connect to the originating server? Or do both preflight /test requests also have to hit the same server for things to work properly?

I am aware that I can add my own session cookie, and hash on its contents. I am currently doing this, but I have two concerns:

  1. Not all users accept cookies, and we may encounter significant hurdles setting 3rd party cookies in our use case.
  2. Browserchannel already uses a sessionId, so adding another is redundant. Adding another id value also seems like unnecessary complexity because it is only weakly associated with the Browserchannel session by way of identifying the useragent (multiple tabs should and do get assigned different SIDs).
josephg commented 10 years ago

The big problem with using the session ID for sticky sessions is that they're set by the server when you connect. Unless your load balancer is listening & remembering the SID property, its too late.

Using OSID could work... The preflight requests don't need to hit the same server, and we could prepopulate OSID with a client-derived value for the initial connection. The problem is that it'll break the ghosting code - when the client reconnects its OSID changes and the original server won't know the client has gone away. That said, the connection will time out naturally after 25 seconds or something anyway, so if you don't mind a little latency in your disconnects that might be fine.

Thinking about it, another solution (perhaps a better solution) would be for the client to set a new header - X-Affinity: <random string> or something. The affinity would be per-tab, and would survive multiple requests (so ghosting would work).You could use consistent hashing on that header and you don't need to worry about EU cookie policy nonsense. The one downside is that I'm not sure if the IE6 code can set custom headers. If you need support for ancient browsers the OSID solution might be better. (Or maybe you could move it to a &affinity=abc123 GET parameter instead).

If you go down this route let me know if you need a hand adding support to the BCSocket client code - it sounds widely useful.

wenzowski commented 10 years ago

RE IE6: setRequestHeader is implemented in the MSXML versions that are prioritized. Is this the IE6 code you're referring to?

josephg commented 10 years ago

Backchannel:

4:49 <wenzowski> I'm not sure what's more widely applicable, an additional GET param or your X-Affinity header idea 
14:49 <wenzowski> using custom load balancer code, so both are equivalent to me 
14:51 <wenzowski> I'm assuming that lb support for JSESSIONID as a GET param would be pretty good, but have no experience to back that up 
14:51 <josephg> I think an additional GET parameter would have better support too 
14:52 <josephg> IE6 (iirc) uses a hidden iframe, and I bet that can't set custom headers 
14:52 <josephg> the xmlhttp implementation you found I think is only used on ie7+ 
14:52 <josephg> - its been awhile since I've touched that code, but thats my recollection. If you're curious enough, I'd suggest firing up a VM and  
                testing it 
14:53 <wenzowski> If I recall correctly, the iframe hack was a way to get around invoking ActiveX, which is what I linked to. Memory fuzzy on that  
                  though. 
14:56 <josephg> yeah... ... I think you should just implement it as an additional GET parameter 
14:56 <wenzowski> I'm not curious enough at the moment since I know the additional GET works 
14:56 <wenzowski> yep 
14:56 <josephg> yep, I agree :) 
14:56 <wenzowski> lol 
14:57 <josephg> also, its really easy to add extra GET parameters 
14:57 <josephg> goog.net.BrowserChannel.prototype.connect = function(testPath, channelPath, opt_extraParams, opt_oldSessionId, opt_oldArrayId) { 
14:57 <josephg> ^--- opt_extraParams is a map, so you should be able to pass {'aff':<string>} 
14:58 <wenzowski> great, thanks 
wenzowski commented 10 years ago

Working on a PR to add an affinity GET var which will contain a client-generated goog.string.getRandomString().