Sockets: Golang rewrite and optimization

[Morfent]

1. The array of sockets is already stored internally in Golang as map[string]*sockjs.session, and I don't want to duplicate that internal map, considering it has the exact shape as what I wanted. Since Golang has the "unsafe" module, the alternative is get the raw pointers for sockjs.session and sockjs.handler and make them free for me to use. Which is the better way of handling this?

2. Unit tests are provided by go test. The also allows benchmarking, which is imperative considering I'll need to compare whether or not sockets.go can handle server load better than the current implementation of socket.js

3. Why don't the static servers support hosting over HTTPS? If there's no problem with it, I could implement it, since Golang can host it using both protocols.

[Zarel]

1. I would prefer to duplicate that map, but if you want to do it yourself, it's not a huge deal.

2. ...ok?

3. they can if you want I guess, we just don't use it for anything

[Morfent]

I would prefer to duplicate that map, but if you want to do it yourself, it's not a huge deal.

I didn't want to keep the additional map since i don't want to risk a desync between the two at any point.

they can if you want I guess, we just don't use it for anything

Ok. I'll keep it, on the rare chance it ends up being used. If not, it's a one-liner to remove.

[Morfent]

This would probably work better as a way to discuss what, if anything could be improved or fixed from the old implementation, considering this issue as is isn't much more than asking for help as it stands.

[Zarel]

Basically the only improvement is to fix bugs in SockJS-node or whatever else is likely to be the problem.

[Morfent]

I think if there are any problems with channels/sockets/subchannels, they should quickly end up apparent. When I wrote the socket multiplexer without mutex, I ended up with sockets deleted before channels, and crashing since the socketID it was given didn't exist. With mutex, that stopped happening. That makes me wary of the current implementation, given it seems to ignore that, say, multiple messages being parsed by a worker at the same time might not always set the correct state.

...though I should really unit test that to check

[Morfent]

The way I thought of dealing with sockets/channels/subchannels ended up working out pretty badly, since only two workers at best could mutate the struct containing them concurrently. Creating a map using the parent's mock worker IDs and the structs I used would allow all workers to run concurrently while still keeping the struct's behaviour, which should be a pretty large improvement over coupling them to the workers like they were in sockets.js.

Internally, Node cluster uses a net server to deal with IPC. In our case, it was using TCP handles, so I'll be using that as well unless there are better alternatives that work cross-platform. In sockets.js, I'll make a WorkerManager class containing the workers map and net server. It along with Worker will contain all state in the file. This makes it possible to unit test sockets.js, since the net server and child process don't get created until Sockets.listen is called.

[Morfent]

SockJS will only be supported on IE8 or IE9 if cookies are enabled, since sockjs-go doesn't support XDR streaming. RFC6455 is the only standard of Websocket protocol supported, so IE6-9, Chrome 6-13, Firefox <10, Safari 5, Opera <12.10, and Konqueror cannot use Websockets for SockJS connections. I may add support for these myself if this becomes a problem.

Config.ssl.options will need to be modified. In Go, HTTPS servers are launched using the paths to the cert and key files, rather than their contents like in Node.

[Zarel]

None of those tradeoffs sound unacceptable. The real question is if it'll fix our major problems.

[Morfent]

What are the major problems you're thinking of?

So far, I've only encountered two problems: race conditions related to Users/Rooms methods that send messages to the child process out of order, and connections not being properly closed if the child process panics. Both are relatively straightforward to solve, though.

[Zarel]

Our biggest problem is probably random disconnections, and also websocket-raw connections that don't close because they have no keepalive system.

[Morfent]

This should fix the deal with websocket-raw connections, since websocket connections in sockets-go use hearbeat messages like every other transport does.

As for the random disconnects, I'm not positive what may be causing them. It could be sockets being prevented from sending heartbeat messages on time because of its worker's IO being starved, websocket-raw transport sockets receiving invalid JSON messages and killing themselves (which they actually do), or just a bug in certain sockjs-node transports.

[Morfent]

I'm thinking of splitting sockets.go into a package of a few files for the extra modularity the file needs badly at the moment and the boost in server startup time from how Go handles compiling packages compared to how it compiles sockets.go now. Making it a package would only make Go compile it if there have been any changes to the package's source code, rather than every single time the server launches like it does now. Would it be alright to do this?

[Zarel]

Sure.

[Morfent]

• The rewrite causes some race conditions that were either already present, but failed silently before, or are caused directly by the rewrite because of slight differences in IPC timing. I'm using this as an excuse to write proper unit tests for more of Sockets/Users/Rooms-related code so I can try to catch them before this would go into production
• I don't want to leave it to the user to manually deal with Go dependency management, since they have no reason to need to care about sockets.js so much just to run PS. I'm thinking of creating scripts to run on postinstall/postupdate hooks to handle that automatically, rather than cramming it all in the launch script to deal with every time or to the user to deal with manually. On the other hand, I could create an npm package to handle that on its own since it's not something PS would ideally concern itself with. What would be more preferable?
• I'll be referring to sockets/channels/subchannels as socket muxing from here since it's less annoying to type

[Morfent]

• I don't want to leave it to the user to manually deal with Go dependency management, since they have no reason to need to care about sockets.js so much just to run PS.

Never mind, this can be worked out for the most part using vendoring. I still want a postinstall script to fetch Go dependencies for the first time, but it shouldn't install Go itself. The launch script can check if any updates to the sockets and main packages I'm splitting sockets.go into have been made so they can be recompiled.

[Zarel]

I would strongly prefer for your code to be able to handle either Go or Node.js sockjs-server processes. We could then default to Node, and then we wouldn't need to support installing Go dependencies automatically.

[Morfent]

That's sounds more reasonable. I'll add a config setting for that

[scheibo]

Is this still something still being worked on, or can it be closed?

SockJS no longer has the perf issues that were plaguing us when I suggested this, making this migration overall no longer worth it.

Originally posted by @Zarel in #2444 (comment)

[scheibo]

Please reopen if this is still being worked on.