elad / node-cluster-socket.io

Writeup on how to make node.js cluster and socket.io play nice
421 stars 63 forks source link

It is not working for me #4

Open xhe opened 10 years ago

xhe commented 10 years ago

I am using your solution, but when I tried to run in browser, it is just hanging, and there is no response at all. My versions are: Node: 0.10.x Express: 4.2.0 redis: 0.10.1 socket.io: 1.0.6 socket.io-redis: 0.1.3

It is exactly same as yours I think this is confusion: process.on('message', function(message, connection) { if (message !== 'sticky-session:connection') { return; } server.emit('connection', connection); }); How to make the real code to process the request, if you are using app.listen(0, 'localhost')

My code is as follows:

var cluster = require('cluster'), net = require('net');

var num_processes = process.env.WORKERS || require('os').cpus().length;

if (cluster.isMaster) { console.log('start cluster with %s workers', num_processes); var workers = []; var spawn = function(i) { workers[i] = cluster.fork();

  workers[i].on('exit', function(worker, code, signal) {
      console.log('respawning worker', i);
      spawn(i);
  });

}; for (var i = 0; i < num_processes; i++) { spawn(i); } var worker_index = function(ip, len) { var s = ''; for (var i = 0, _len = ip.length; i < _len; i++) { if (ip[i] !== '.') { s += ip[i]; } } return Number(s) % len; }; var server = net.createServer(function(connection) { var worker = workers[worker_index(connection.remoteAddress, num_processes)]; worker.send('sticky-session:connection', connection); }).listen(3000); } else {

var init = require('./config/init')(),
config = require('./config/config'),
mongoose = require('mongoose');
var sio = require('socket.io');
var sio_redis = require('socket.io-redis');

//Bootstrap db connection
var db = mongoose.connect(config.db);
//Init the express application
var app = require('./config/express')(db);

//Start the app by listening on <port>
var server = app.listen(0, 'localhost');

var io = sio(server);
io.adapter(sio_redis({ host: 'localhost', port: 6379 }));

var socketService = require('./app/services/sockets');
socketService()['init'](io);

// Listen to messages sent from the master. Ignore everything else.
process.on('message', function(message, connection) {
     if (message !== 'sticky-session:connection') {
        return;
    }

    server.emit('connection', connection);
});

//Logging initialization
console.log('MEAN.JS application started on port ' + config.port);

}

process.on('uncaughtException', function (err) { console.error((new Date).toUTCString() + ' uncaughtException:', err.message) console.error(err.stack) process.exit(1) })

vicary commented 9 years ago

I am having the same issue, even with the original sticky-session.

It seems that net connections passed from master to children is not released properly, choking the backlog with unreleased net connections (fd).

elad commented 9 years ago

You are correct. This is an issue with node.js itself, see #7784 and #7905.

Three days ago @cjihrig posted a fix in #8576, hopefully it will be merged soon and a new release with the fix tagged shortly thereafter. :)

vicary commented 9 years ago

Do you think anywhere I can patch the readStart() at runtime? https://github.com/joyent/node/issues/7905#issuecomment-54211291

I want to make my app work ASAP before the official fix.

samcday commented 9 years ago

@vicary the workaround I outlined in joyent/node#7905 does work okay:

server.on("connection", function(c) {
    c._handle.readStop();
});
vicary commented 9 years ago

Thanks for that, works like a charm. >95% of connections are running smooth now. :smile:

samcday commented 9 years ago

95%

webscale. :P

elad commented 9 years ago

This was just fixed in the v0.12 branch. I have no idea when it will be released, but once it happens I'll update the document to reflect what has to change. (Should be using the pauseOnConnect option and resume the socket in the child.)

barisusakli commented 9 years ago

I am looking into implementing this, @samcday where does that snippet go? I am assuming it goes into master to stop it from reading anything before being passed to the worker.

vicary commented 9 years ago

I've chopped down my master.js for your reference. https://gist.github.com/vicary/b8a7664227ec0245a0dc

Not sure if the cluster.js part, which I took reference from node.js core, is necessary, you may experiment yourself.

EDIT A more simple workaround is to add c._handle.readStop() at line 51 at sticky-session.js.

barisusakli commented 9 years ago

Thanks @vicary, I can't use sticky-session.js since I already have code that has clustering in a separate file.

Is the handle closing code 100% necessary in your sample? https://gist.github.com/vicary/b8a7664227ec0245a0dc#file-master-js-L74-L81

Also when do you send the sticky.accept from the worker?

vicary commented 9 years ago

In short, I'm not sure.

Like what I wrote in the comments, I've crunched through the node.js core, I can assure you that's how node handles cluster on itself. But sticky-session totally bypasses that round robin hand off logic and creates a net server instead. You may experiment yourself for either case.

Here is an example of worker.js

barisusakli commented 9 years ago

Yeah after I implemented it, it works on my test environment although with the handle closing code the handles array grows and has empty elements in it which is excepted since seq just keeps growing.

closing 2
[object TCP],[object TCP],
closing 3
[object TCP],[object TCP],,,[object TCP],[object TCP],[object TCP]
closing 4
[object TCP],[object TCP],,,,[object TCP],[object TCP],[object TCP]
closing 5
[object TCP],[object TCP],,,,,[object TCP],[object TCP]
closing 6
[object TCP],[object TCP],,,,,,[object TCP]
closing 7
[object TCP],[object TCP],,,,,,
closing 8
[object TCP],[object TCP],,,,,,,
closing 9
[object TCP],[object TCP],,,,,,,,
closing 10
[object TCP],[object TCP],,,,,,,,,

This is when I reload the page. Not sure how this will behave under load where lots of connections are coming in. handles will just keep growing.

barisusakli commented 9 years ago

I changed the handles = [] to handles={} seems to work fine as well and it doesn't result in an array with lots of empty elements.

vicary commented 9 years ago

The seq thing is how node handles their connections, AFAIK that doesn't affects memory as long as you delete it. With {} it is actually a tiny bit slower because you are extending an object every time you create a new property name.

Doesn't really matter tho, the difference is in the scale of nanoseconds.

numannaufal commented 7 years ago

Hi, I am beginner for Node.js clustering. I followed example code step by step but doesn't work. Very appreciate if you could help me.

Environment: Node: 7.10.0. Redis: 3.2.9 { "express": "^4.15.2", "socket.io": "^1.7.3", "socket.io-redis": "^5.1.0", }

this is my code.

require('dotenv').config();
var logger = require('./app/utils/logger')('index.js');
var sticky = require('sticky-session');
var express = require('express');
var controller = require('./app/controllers/indexController');
var serverPort = 9998;
var net = require('net');
var cluster = require('cluster');
var numCPUs = require('os').cpus().length;
var sio = require('socket.io');
var sio_redis = require('socket.io-redis');

if (cluster.isMaster) runClusterMaster();
else runClusterFork();

function runClusterMaster() {
    console.log(`Master ${process.pid} is running`);
    var workers = [];
    var spawn = function(i) {
        workers[i] = cluster.fork();
        workers[i].on('exit', function(code, signal) {
            console.log('worker exit', i);
        });
    };

    for (let i = 0; i < numCPUs; i++) {
        spawn(i);
    }

    var worker_index = function(ip, len) {
        var s = '';
        for (var i = 0, _len = ip.length; i < _len; i++) {
            if (!isNaN(ip[i])) {
                s += ip[i];
            }
        }
        return Number(s) % len;
    };

    var server = net.createServer({ pauseOnConnect: true }, function(connection) {
        var worker = workers[worker_index(connection.remoteAddress, numCPUs)];
        worker.send('sticky-session:connection', connection);
    }).listen(serverPort);
    uncaughtTheUncaught();
}

function runClusterFork() {
    var app = express(); 
    app.use(express.static('public'));
    var server = app.listen(0, 'localhost');
    var io = sio(server);
    io.adapter(sio_redis({ host: 'localhost', port: 6379 }));
    controller(app, io);

    process.on('message', function(message, connection) {
        if (message !== 'sticky-session:connection') {
            return;
        }
        server.emit('connection', connection);
        connection.resume();
    });
    uncaughtTheUncaught();
}

function uncaughtTheUncaught() {
    process.on('uncaughtException', (err) => {
        logger.log('uncaught', err)
    });

    process.on('SIGINT', function() {
        process.exit();
    });

    process.on('exit', function (){
        console.log('Goodbye!');
    });
}

The problem is: Sample feature: real time comments. Given client in browser A. Given client in browser B. (When not using cluster is fine) When using cluster: (A) post comment -> (A) itself could see the changes (receive the socket message), but (B) cannot. or sometimes (A) post comment -> (A) itself couldn't see the changes (doens't receive socket message), but (B) can.