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

Research/Documentation: sails.sockets.* methods in a multi-node environment #6457

Closed
mikermcneil opened this issue Jan 11, 2016 · 17 comments
Closed

Comments

@mikermcneil
Copy link
Member

To start with, we need to verify:

  • whether or not .emit() in socket.io truly works in a multi-node environment (my understanding presently is that it works sometimes depending on server configuration (see http://socket.io/docs/using-multiple-nodes/#sticky-load-balancing)
    • (and what Guillermo's plans are for it long term)
  • whether or not .subscribers() works in a multi-node environment (pretty sure this one is always a "no" because of how the socket.io redis adapter works)
    • (and what Guillermo's plans are for it long term)

Originally posted in:
#3008 (comment)

@sgress454
Copy link
Member

After many careful deliberations, we've come to the following conclusions:

  • .emit() does not work cross-server; that is, you cannot use .emit() to send a message to a socket connected to another node.
  • .subscribers() does not work cross-server; it will only return the IDs of sockets connected to the node on which it was called. Furthermore, even if it did work, it would be of questionable use, since a node cannot directly manipulate sockets connected to other nodes.
  • .broadcast() does work cross server, because each connected socket is automatically subscribed to a room with its ID as the name. Thus, you can do .broadcast(socketId, event) from any node, and the socket with the specified ID will receive the message.
  • What we really need is a way to use .join() and .leave() cross-server, so that with one call in app-level code we can make all sockets currently subscribed to room A go and join room B. This is currently implemented in a patchwork way via .subscribers() and the admin bus, allowing Sails resourceful PubSub to work cross-server.

The proposal we'll be moving forward with is to implement three new methods in the sails.sockets namespace, tentatively named:

  • addRoomMembersToRoom(sourceRoom, destRoom): takes all members of sourceRoom on all nodes and subscribes them to destRoom.
  • removeRoomMembersFromRoom(sourceRoom, destRoom): takes all members of sourceRoom on all nodes and unsubscribes them from destRoom.
  • leaveAll(sourceRoom): takes all members of sourceRoom on all nodes and unsubscribes them from all rooms.

These will be implemented using a refactored admin bus that communicates with Redis directly, rather than connecting via socket.io-client. The current implementation provides a nice abstraction, but has some maintainability tradeoffs that don't seem worth it considering the lack of any other viable socket.io adapters besides socket.io-redis. The upshot is that in app code, you will be able to do:

sails.sockets.addRoomMembersToRoom(roomA, roomB)

to subscribe all sockets in room A to room B, regardless of what node the sockets are connected to.

Of the methods currently in sails.sockets, we will be deprecating all but .blast(), .broadcast(), .getId(), .join() and .leave(). You will still have access to the underlying socket.io instance as sails.io if you need more, but it's our belief that anything you want to do in a Sails app you should be able to do with those methods, and restricting that set makes it easier to build an app that works reliably in a clustered environment.

@mikermcneil
Copy link
Member Author

Well said 👏

@mikermcneil
Copy link
Member Author

@sgress454 re: join and leave, while the recommended usage should become the socket id (to encourage a purely multi-node API), I think we can safely leave support for passing in req without showing a deprecation message. It'd work just like it does now, and just like the case we were talking about earlier re: the optimization which first checks whether the specified socket id is available on the local server (minus the lookup to actually get the instance, since we'd already have it).

sgress454 referenced this issue in balderdashy/sails-hook-sockets Jan 14, 2016
…methods

Added code in admin bus handler to forward certain messages to those functions
refs #19
@sgress454
Copy link
Member

The refactored admin bus has been published with v0.13.2. The new methods are:

sails.sockets.addRoomMembersToRooms(sourceRoom, destRooms, cb)
sails.sockets.removeRoomMembersFromRooms(sourceRoom, destRooms, cb)
sails.sockets.leaveAll(sourceRoom, options, cb)

All are documented in the 0.12 branch of sails-docs.

In addition, .join() and .leave() now work cross-server.

I also refactored the .introduce() and .retire() pubsub methods in Sails to make use of these new functions instead of using the admin bus directly.

The one caveat with all this is that in multi-server environments, you'll want to do a setTimeout after something like join or addRoomMembersToRooms if you plan on broadcasting immediately afterwards, to give the other servers in the cluster a chance to act. This is an issue w/ clusters in general, and a definitive solution (if there is such a thing) involves a lot of extra cross-server communication and collation that seems like overkill for our use case. I'll add some notes in the docs about setTimeout where relevant.

@mikermcneil
Copy link
Member Author

@sgress454 I'd rather we avoid recommending setTimeout, and instead explain the situation in notes (eg join/leave are not guaranteed to have taken effect- or are "volatile" in the same way as broadcast)

@rauchg
Copy link

rauchg commented Jan 15, 2016

whether or not .emit() in socket.io truly works in a multi-node environment (my understanding presently is that it works sometimes depending on server configuration (see http://socket.io/docs/using-multiple-nodes/#sticky-load-balancing)

You just need something to broker the messages. socket.io-redis for example allows you to emit from anywhere in your backend with just one line of code: https://github.com/socketio/socket.io-emitter

var io = require('socket.io-emitter')({ host: '127.0.0.1', port: 6379 });
setInterval(function(){
  io.emit('time', new Date);
}, 5000);

Sticky load balancing is of course required if you use long polling, which is up to you to support. If you want greater support of clients and networks, you use long polling, which requires that messages are "tied" in some way.

@rauchg
Copy link

rauchg commented Jan 15, 2016

Something important to mention: we're very interested in expanding the capabilities of the adapter so that subscriptions to rooms can also be managed "remotely" in the same way you can "emit" remotely.

@mikermcneil
Copy link
Member Author

Thanks @rauchg!

We've added basic mention of sticky load balancing to our docs since we upgraded to sio 1.0, but I think there's def room for more conceptual documentation on what that means in the context of socket.io. If you know any good tutorials you trust about setting up the lb ( nginx/haproxy or better yet for heroku/azure/AWS) I'd be down to put together a coalesced tutorial. I'd say it's the biggest source of confusion for folks when they're going to production, just because it's usually a new thing they haven't had to deal with before.

@mikermcneil mikermcneil reopened this Jan 15, 2016
@sgress454
Copy link
Member

@rauchg re: emit, it was more of a question of whether sails.sockets.emit() worked cross-server--this is a wrapper method we created that allowed you to supply a socket id as an argument, which would then be used to look up the appropriate socket instance and call its .emit() method. This didn't work cross-server, since server A doesn't have access to server B's socket instances. But broadcasting works beautifully for this purpose, since each socket is automatically joined to a room with its ID as the name.

re: expanding the adapter capabilities--sounds great! FWIW, the solution we implemented would be easy to incorporate directly into socket.io-redis--basically setting up a separate pub/sub channel that the servers can use to communicate with each other, and forward commands like join around. See https://github.com/balderdashy/sails-hook-sockets/blob/master/lib/connect-to-admin-bus.js for details.

@mikermcneil
Copy link
Member Author

@sgress454 reckon we're ready to upgrade internally and start trying this out in production in an rc?

@sgress454
Copy link
Member

@mikermcneil sure!

@JamesMessinger
Copy link

I'm currently experiencing this problem, or something pretty close to it, and am looking for a solution. All socket rooms are empty when using socket.io-redis, so I'm unable to send messages to clients via any of the sails.sockets.* methods — not even via sails.sockets.broadcast().

@sgress454 - Did I understood your comment above correctly, that this issue has been fixed in the latest v0.12 version? If so, then maybe I'm doing something wrong, because I'm still experiencing the problem in v0.12.0-rc8.

@sgress454
Copy link
Member

@BigstickCarpet This issue was opened to explore the use of things like .emit() and .subscribers() cross-server; most of the sails.sockets.* methods like .join() and .broadcast() have been working fine since 0.11.x. The tests are also passing, so I'm inclined to think (hope?) that it's an implementation issue on your end. With that in mind, if you're still having problems please first read the contribution guide about opening an issue, and then open a new issue following those guidelines so we can help track down the bug if there is one!

@mikermcneil
Copy link
Member Author

@sgress454
Copy link
Member

v0.12 of Sails has been released along with new docs, including a section on realtime programming with multi-server environments. I think we can put this one to bed!

@crobinson42
Copy link

RE: deprecated .rooms()
If there is an effort to support .broadcast() & .join()/.leave() in a multi-node app, why not also include .rooms() (and also .subscribers())?

http://sailsjs.org/documentation/reference/web-sockets/sails-sockets/sails-sockets-rooms
^ recommends keeping track of rooms in app level code which in a multi-node environment would require the use of redis, mongo, or some other method of persistence. It seems like sails is doing this already with .join() and the other accompanying methods... why not .rooms()?

@mikermcneil
Copy link
Member Author

@crobinson42 we recommend this because room crud is something that almost always includes a need to store additional info-- eg the timestamp when the room was created, when sockets joined it, etc. (Ends up being cleaner to implement with your models)

@johnabrams7 johnabrams7 transferred this issue from balderdashy/sails-hook-sockets May 21, 2019
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

5 participants