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.

Sign up for GitHub

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

Jump to bottom

fix: should stop app when baseDir is symlink #12

Merged
merged 1 commit into from
Nov 14, 2017
Merged

fix: should stop app when baseDir is symlink #12

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/cmd/stop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class StopCommand extends Command {
// node /Users/tz/Workspaces/eggjs/egg-scripts/lib/start-cluster {"title":"egg-server","workers":4,"port":7001,"baseDir":"/Users/tz/Workspaces/eggjs/test/showcase","framework":"/Users/tz/Workspaces/eggjs/test/showcase/node_modules/egg"}
let processList = yield this.helper.findNodeProcess(item => {
const cmd = item.cmd;
return cmd.includes(this.serverBin) && cmd.includes(`"baseDir":"${baseDir}"`);
return cmd.includes('start-cluster');
Copy link
Member

Choose a reason for hiding this comment

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

这里之前也是纠结要不要严谨一点，否则 window 的用 find-process 就很好搞了。

这里不判断目录的话，会 kill 掉其他 egg 应用吧，有些场景是一个机器多个 egg，没 docker 隔离的。

Copy link
Member Author

@popomore popomore Nov 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

隔离的到时可以加个唯一 id 做标识，我们应该不推崇这种方式，支持部署脚本支持一下。

Copy link
Member

Choose a reason for hiding this comment

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

至少判断下 baseDir 的 basename?

Copy link
Member Author

Choose a reason for hiding this comment

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

如果真是软连接，basename 也不一定一样

Copy link

Choose a reason for hiding this comment

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

追加个port值是否更精确呢

Copy link

Choose a reason for hiding this comment

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

你指的是worker的监听端口会随着重启而不断变化么？ 不过master的端口是固定的吧

Copy link
Member

Choose a reason for hiding this comment

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

master 不提供 http 服务，无端口。

Copy link

Choose a reason for hiding this comment

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

那监听0.0.0.0：7001端口的node进程（并且包含多个子进程）不是master进程？

Copy link

Choose a reason for hiding this comment

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

node 9.2 支持 process.ppid
nodejs/node#16839

Copy link
Member

@atian25 atian25 Nov 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

master - agent - workerS(7001)

https://eggjs.org/zh-cn/core/cluster-and-ipc.html

});
let pids = processList.map(x => x.pid);

Expand All @@ -55,7 +55,7 @@ class StopCommand extends Command {
// node /Users/tz/Workspaces/eggjs/test/showcase/node_modules/[email protected]@egg-cluster/lib/app_worker.js {"framework":"/Users/tz/Workspaces/eggjs/test/showcase/node_modules/egg","baseDir":"/Users/tz/Workspaces/eggjs/test/showcase","port":7001,"workers":2,"plugins":null,"https":false,"key":"","cert":"","title":"egg-server","clusterPort":52406}
processList = yield this.helper.findNodeProcess(item => {
const cmd = item.cmd;
return cmd.includes(`"baseDir":"${baseDir}"`) && (cmd.includes('app_worker.js') || cmd.includes('agent_worker.js'));
return cmd.includes('egg-cluster/lib/app_worker.js') || cmd.includes('egg-cluster/lib/agent_worker.js');
});
pids = processList.map(x => x.pid);

Expand Down