feat:add sticky cluster mode #14

ngot · 2017-02-09T10:48:37Z

Checklist

npm test passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

Description of change

增加sticky模式，用于websocket服务模式

mention-bot · 2017-02-09T10:48:38Z

@ngot, thanks for your PR! By analyzing the history of the files in this pull request, we identified @dead-horse, @popomore and @atian25 to be potential reviewers.

codecov · 2017-02-09T10:54:10Z

Codecov Report

Merging #14 into master will increase coverage by 0.33%.

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
+ Coverage   96.25%   96.58%   +0.33%     
==========================================
  Files           7        7              
  Lines         267      293      +26     
==========================================
+ Hits          257      283      +26     
  Misses         10       10

Impacted Files	Coverage Δ
lib/app_worker.js	`96.66% <100%> (+1.01%)`	✅
lib/master.js	`95.58% <100%> (+0.51%)`	✅

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d988b42...828280a. Read the comment docs.

fengmk2 · 2017-02-09T11:09:02Z

参考来源？

ngot · 2017-02-09T11:23:35Z

主要参考这个做的 https://github.com/elad/node-cluster-socket.io

ngot · 2017-02-09T11:25:26Z

http://socket.io/docs/using-multiple-nodes/
https://github.com/indutny/sticky-session

ngot · 2017-02-10T05:32:55Z

@popomore @fengmk2 @dead-horse 同志们帮忙review下

fengmk2 · 2017-02-10T06:47:46Z

改动有点大，需要点时间去消耗一下什么是 sticky

fengmk2 · 2017-02-10T06:48:51Z

lib/app_worker.js

@@ -34,7 +34,22 @@ function startServer() {
  // emit `server` event in app
  app.emit('server', server);

-  server.listen(options.port);
+  if (options.sticky) {
+    server.listen(0, '127.0.0.1');


这一步 listen 有什么作用？

原来的cluster模式完全没有改变。每个worker内的http的hander，依然需要存在，用来接受master转发过来的socket对象，来进行下一步的处理。这里listen 0只是为了启动http handler，而不暴露出去。

去掉这个 listen 应该也是可以的。

需要listen，不然逻辑走不下去

fengmk2 · 2017-02-10T07:37:33Z

lib/master.js

+    }).listen(this.options.port);
+  }
+
+  stickyWorker(ip) {


要写一个测试，看看 stickyWorker 这个算法是否分配均匀

这个算法，是根据用户的IP和进程数量做的一个hash匹配。能够保证，某个固定ip映射到某个固定Worker。但是其实，并不能完全保证平均分配。如果，实际访问的样本IP是平均分布的，那么问题不大，如果是集中某几个固定IP，那么就会导致非常不均衡。昨天，想到公司内部做过代理，导致来源IP被篡改为公司固定的出口IP，是有问题的。

fengmk2 · 2017-02-10T07:41:58Z

lib/master.js

@@ -309,6 +343,11 @@ class Master extends EventEmitter {
    }

    this.isAllAppWorkerStarted = true;
+
+    if (this.options.sticky) {
+      this.startMasterSocketServer();


startMasterSocketServer 也是一个异步过程，它成功了才能 ready 成功

return this.startMasterSocketServer(err => { if (err) return this.ready(err); this.ready(true); });

fengmk2 · 2017-02-10T07:46:02Z

test/master.test.js

+      app.ready(done);
+    });
+
+    after(() => {


app.close() 返回的是 promise，需要 return 才会生效，写成 after(() => app.close()) 吧

fengmk2 · 2017-02-10T07:46:19Z

test/master.test.js

+  describe('--sticky', () => {
+    before(done => {
+      app = utils.cluster('apps/cluster_mod_sticky', { sticky: true });
+      app.ready(done);


return app.ready() 即可，不需要 done 了

fengmk2

LGTM

atian25 · 2017-02-10T07:54:17Z

lib/app_worker.js

+  if (options.sticky) {
+    server.listen(0, '127.0.0.1');
+    // Listen to messages sent from the master. Ignore everything else.
+    process.on('message', (message, connection) => {


忽略其他消息这个会不会影响到 messeger 那边?

应该不会，这个只是新增一个listenser，只限定在这个逻辑，不会影响其他的。

dead-horse · 2017-02-10T07:55:16Z

lib/master.js

@@ -58,8 +58,12 @@ class Master extends EventEmitter {

    this.ready(() => {
      this.isStarted = true;
-      this.logger.info('[master] %s started on %s://127.0.0.1:%s (%sms)',
-        frameworkPkg.name, this.options.https ? 'https' : 'http', this.options.port, Date.now() - startTime);
+      let stickyMsg = '';


const stickMsg = this.options.sticky ? ' with STICKY MODE!' : '';

dead-horse · 2017-02-10T08:00:48Z

lib/master.js

+    const wsd = ws.map(w => w % workerNumbers);
+
+    let s = '';
+    for (let i = 0, _len = ip.length; i < _len; i++) {


i < ip.length 就可以了

dead-horse · 2017-02-10T08:01:05Z

lib/master.js

+        s += ip[i];
+      }
+    }
+    s = +s;


s = Number(s) 吧

dead-horse · 2017-02-10T08:02:40Z

lib/master.js

+
+    let s = '';
+    for (let i = 0, _len = ip.length; i < _len; i++) {
+      if (!isNaN(ip[i])) {


这里是不是用一些 hash 算法会比较好一点？不然很容易就造出一系列 ip 地址全部都连接到一个 worker 上了。

嗯，是的，我也有这个担忧

codecov · 2017-02-10T09:18:09Z

Codecov Report

Merging #14 into master will increase coverage by 0.33%.

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
+ Coverage   96.25%   96.58%   +0.33%     
==========================================
  Files           7        7              
  Lines         267      293      +26     
==========================================
+ Hits          257      283      +26     
  Misses         10       10

Impacted Files	Coverage Δ
lib/master.js	`95.58% <100%> (+0.51%)`	✅
lib/app_worker.js	`96.66% <100%> (+1.01%)`	✅

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d988b42...9deb273. Read the comment docs.

fengmk2 · 2017-02-10T11:27:01Z

lib/master.js

+        s += ip[i];
+      }
+    }
+    s = Number(s);


那直接改成 hash(ip) % workerNumbers ?

我研究看看，有没有更好些的算法

框架本身的话，没有找到很好的算法。大多数做法，还是在集群前面架设负载设备来做。目前，就这样其实就已经够了。

fengmk2 · 2017-02-13T10:25:54Z

LGTM, need @dead-horse one more +1

dead-horse · 2017-02-13T10:51:14Z

+1, 先这样，到时候文档里面说清楚这个模式下的一些弊端和怎样配合反向代理使用好了。cluster 的文档里面都可以不介绍这个功能，只在 socket.io 的文档里面说明好了？

dead-horse · 2017-02-13T10:54:42Z

1.4.0

ngot · 2017-02-14T03:24:50Z

就只在socket.io说明就好了

ngot added 2 commits February 9, 2017 18:44

feat:add sticky cluster mode

98f2e68

doc:add sticky mod doc

7f14655

ngot mentioned this pull request Feb 9, 2017

[RFC] egg-socket.io eggjs/egg#269

Closed

fengmk2 reviewed Feb 10, 2017

View reviewed changes

fengmk2 added the enhancement label Feb 10, 2017

fengmk2 requested a review from dead-horse February 10, 2017 07:51

fengmk2 approved these changes Feb 10, 2017

View reviewed changes

atian25 reviewed Feb 10, 2017

View reviewed changes

dead-horse reviewed Feb 10, 2017

View reviewed changes

ngot added 2 commits February 10, 2017 17:17

fix:shoud get ready after master server start

2cd7d0a

test:replace done with return promise

828280a

chore:simplify log

9deb273

fengmk2 reviewed Feb 10, 2017

View reviewed changes

dead-horse approved these changes Feb 13, 2017

View reviewed changes

dead-horse merged commit 4a51254 into master Feb 13, 2017

dead-horse deleted the add-sticky-mod branch February 13, 2017 10:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat:add sticky cluster mode #14

feat:add sticky cluster mode #14

ngot commented Feb 9, 2017 •

edited

Loading

mention-bot commented Feb 9, 2017

codecov bot commented Feb 9, 2017 •

edited

Loading

fengmk2 commented Feb 9, 2017

ngot commented Feb 9, 2017

ngot commented Feb 9, 2017

ngot commented Feb 10, 2017

fengmk2 commented Feb 10, 2017

fengmk2 Feb 10, 2017

ngot Feb 10, 2017

fengmk2 Feb 10, 2017

ngot Feb 10, 2017

fengmk2 Feb 10, 2017

ngot Feb 10, 2017

fengmk2 Feb 10, 2017

fengmk2 Feb 10, 2017

fengmk2 Feb 10, 2017

fengmk2 left a comment

atian25 Feb 10, 2017

ngot Feb 10, 2017

dead-horse Feb 10, 2017

dead-horse Feb 10, 2017

dead-horse Feb 10, 2017

dead-horse Feb 10, 2017

ngot Feb 10, 2017

codecov bot commented Feb 10, 2017 •

edited

Loading

fengmk2 Feb 10, 2017

ngot Feb 10, 2017

ngot Feb 13, 2017 •

edited

Loading

fengmk2 commented Feb 13, 2017

dead-horse commented Feb 13, 2017

dead-horse commented Feb 13, 2017

ngot commented Feb 14, 2017

feat:add sticky cluster mode #14

feat:add sticky cluster mode #14

Conversation

ngot commented Feb 9, 2017 • edited Loading

Checklist

Affected core subsystem(s)

Description of change

mention-bot commented Feb 9, 2017

codecov bot commented Feb 9, 2017 • edited Loading

Codecov Report

fengmk2 commented Feb 9, 2017

ngot commented Feb 9, 2017

ngot commented Feb 9, 2017

ngot commented Feb 10, 2017

fengmk2 commented Feb 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fengmk2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 10, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngot Feb 13, 2017 • edited Loading

Choose a reason for hiding this comment

fengmk2 commented Feb 13, 2017

dead-horse commented Feb 13, 2017

dead-horse commented Feb 13, 2017

ngot commented Feb 14, 2017

ngot commented Feb 9, 2017 •

edited

Loading

codecov bot commented Feb 9, 2017 •

edited

Loading

codecov bot commented Feb 10, 2017 •

edited

Loading

ngot Feb 13, 2017 •

edited

Loading