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

feat: support single process mode #3430

Merged
merged 5 commits into from
Feb 3, 2019
Merged

feat: support single process mode #3430

merged 5 commits into from
Feb 3, 2019

Conversation

dead-horse
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Jan 26, 2019

Codecov Report

Merging #3430 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3430      +/-   ##
==========================================
+ Coverage   99.76%   99.78%   +0.01%     
==========================================
  Files          29       32       +3     
  Lines         844      911      +67     
==========================================
+ Hits          842      909      +67     
  Misses          2        2
Impacted Files Coverage Δ
lib/egg.js 100% <100%> (ø) ⬆️
lib/core/messenger/index.js 100% <100%> (ø)
lib/start.js 100% <100%> (ø)
lib/core/messenger/ipc.js 100% <100%> (ø)
lib/core/messenger/local.js 100% <100%> (ø)
...ps/master-worker-started/node_modules/egg/index.js 100% <0%> (ø) ⬆️

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 e899630...cb5ca3d. Read the comment docs.

const EventEmitter = require('events');

/**
* Communication between app worker and agent worker by IPC channel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

注释也要修改一下

opposite = application;
}

// use nextTick to keep it asyn as IPC messenger
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asyn => async

const Agent = require('./agent');

module.exports = async (options = {}) => {
options.baseDir = options.baseDir || process.cwd();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里考虑加入 timeout 参数, timeout 之后需要 close agent 和 application。

@dead-horse
Copy link
Member Author

绝大部分和 cluster 模式兼容,但是仍然有无法兼容的地方,需要文档中列出来:

  • messenger.sendTo(pid, action, data) 单进程模式下只能发送给 process.pid,效果相当于 broadcast,可以考虑整体 deprecate 这个 API,不建议用户使用。
  • app 上监听不到 server 事件,server 由用户自己来创建

开发环境(egg-development)需要适配,由于没有 egg-cluster,需要看看怎么实现热重启(或者不做热重启,和 koa 一样交给用户处理)

@fengmk2
Copy link
Member

fengmk2 commented Jan 28, 2019

app 上监听不到 server 事件,server 由用户自己来创建

这个不好吧,都提供 listen 接口了,为何不能拿到 server 事件。

@dead-horse
Copy link
Member Author

这个不好吧,都提供 listen 接口了,为何不能拿到 server 事件。

和 koa 一样,用户可以 require('http').createServer(app.callback()),这样是拿不到的

lib/start.js Outdated

module.exports = async (options = {}) => {
options.baseDir = options.baseDir || process.cwd();
options.singleProcess = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options.mode=cluster/single 怎么样?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对外可以考虑统一一个 start(options) 的入口,然后里面可以转调 startCluster,这样的话 egg-bin 和 egg-scripts 那边不用判断 options

@atian25
Copy link
Member

atian25 commented Jan 28, 2019

messenger 这还有个场景,之前的用法里面,进程是可以直接 process.send()

@dead-horse
Copy link
Member Author

messenger 这还有个场景,之前的用法里面,进程是可以直接 process.send() 的

文档有写这个用法么?

@atian25
Copy link
Member

atian25 commented Jan 28, 2019

messenger 这还有个场景,之前的用法里面,进程是可以直接 process.send() 的

文档有写这个用法么?

Guide 文档没有写,在 egg-cluster 的 messenger 源码有,然后很多地方的单元测试好像都这么用

@dead-horse
Copy link
Member Author

恩,这个问题不大,文档说明一下好了

@atian25
Copy link
Member

atian25 commented Jan 30, 2019

这个完成后,本地开发可以直接用单进程模式了吧,egg-bin debug 那块就能简单多了。

const Agent = require('./agent');

module.exports = async (options = {}) => {
console.warn('single process mode is still in experiment, please don\'t use it in production environment');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

先加一个 warn 信息,发个版本,用来调试

@dead-horse dead-horse changed the title [WIP] feat: support single process mode feat: support single process mode Feb 3, 2019
@dead-horse dead-horse merged commit 20ba463 into master Feb 3, 2019
@dead-horse dead-horse deleted the all-in-one branch February 3, 2019 04:41
popomore pushed a commit that referenced this pull request Feb 3, 2019
feat: support single process mode (#3430)
const Application = require('./application');
const Agent = require('./agent');

module.exports = async (options = {}) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我最终想是在 egg-cluster 实现的,这样 API 也比较统一

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

一开始我也以为会在 cluster 做。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为啥要在 egg-cluster 实现? egg 就应该是一个可以自运行的服务,和 koa 一样

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

egg-cluster 像 pm2,是 egg 的进程管理工具。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

主要是现在 chair-script, egg-script 里都写死调用的是 startCluster。

现在 faas 那边,我是自己覆盖了 startCluster 来搞的

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

egg-cluster 也可以增加一个参数直接调用 egg 的 start 方法。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我记得当时那个 PR 里面我提过,startCluster 里面判断下调用的

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同意死马,egg 应该不依赖 cluster 就能够启动。单进程模式,应该推荐直接初始化 egg 的实例就来启动服务的。在容器里面运行,甚至都不要 cfork,egg 进程死掉的话,交给 k8s 来重启。

不过直接干掉 agent 可能会引起一些插件不兼容。

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同意死马,egg 应该不依赖 cluster 就能够启动。单进程模式,应该推荐直接初始化 egg 的实例就来启动服务的。在容器里面运行,甚至都不要 cfork,egg 进程死掉的话,交给 k8s 来重启。

不过直接干掉 agent 可能会引起一些插件不兼容。

赞同

@ManfredHu
Copy link

可以问下这里单进程是啥进度了吗?调用方式是?有文档吗?
自己试了下warning如下,预计什么时间可以正式使用呢?
single process mode is still in experiment, please don't use it in production environment

@atian25
Copy link
Member

atian25 commented May 15, 2019

我们内部才开始实践中,要下半年吧

@fengmk2
Copy link
Member

fengmk2 commented Jun 6, 2023

已经用起来了!

@atian25
Copy link
Member

atian25 commented Jun 6, 2023

已经用起来了!

warning 我们干掉了么?

@fengmk2
Copy link
Member

fengmk2 commented Jun 6, 2023

已经用起来了!

warning 我们干掉了么?

感觉可以。

SEWeiTung added a commit to SEWeiTung/egg that referenced this pull request Jun 23, 2023
Since we're now successfully using this feature for a long time without
obvious bugs/errors, there's no need for us to make it as an
"experinment" feature and now it's come to ture.

Ref: eggjs#3430
fengmk2 pushed a commit that referenced this pull request Jun 23, 2023
Since we're now successfully using this feature for a long time without
obvious bugs/errors, there's no need for us to make it as an
"experinment" feature and now it's come to ture.

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

Successfully merging this pull request may close these issues.

8 participants