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

It is not working for me #4

Open
xhe opened this issue Sep 4, 2014 · 15 comments
Open

It is not working for me #4

xhe opened this issue Sep 4, 2014 · 15 comments

Comments

@xhe
Copy link

xhe commented Sep 4, 2014

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
Copy link

vicary commented Oct 21, 2014

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
Copy link
Owner

elad commented Oct 21, 2014

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
Copy link

vicary commented Oct 22, 2014

Do you think anywhere I can patch the readStart() at runtime? nodejs/node-v0.x-archive#7905 (comment)

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

@samcday
Copy link

samcday commented Oct 22, 2014

@vicary the workaround I outlined in nodejs/node-v0.x-archive#7905 does work okay:

server.on("connection", function(c) {
    c._handle.readStop();
});

@vicary
Copy link

vicary commented Oct 22, 2014

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

@samcday
Copy link

samcday commented Oct 22, 2014

95%

webscale. :P

@elad
Copy link
Owner

elad commented Oct 27, 2014

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
Copy link

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
Copy link

vicary commented Nov 26, 2014

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
Copy link

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
Copy link

vicary commented Nov 27, 2014

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
Copy link

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
Copy link

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
Copy link

vicary commented Nov 28, 2014

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
Copy link

numannaufal commented Jun 30, 2017

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.

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

No branches or pull requests

6 participants