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

add new types of node lock #554

Merged
merged 1 commit into from
Mar 9, 2022
Merged

Conversation

DuodenumL
Copy link
Contributor

之前remap要拿一个node的锁,而且remap流程还挺长的,一直拿着锁会卡住其它部署请求。而remap的锁只是为了保证不会有多个请求同时去remap,其实可以用chan来代替锁的功能。

修改之后会有一个goroutine来循环监听remapChan,调用方只需要异步将node name塞进remapChan即可,大大节约了持有锁的时间。

为了尽量防止脏数据的问题,remap里的实现也稍微改了下,每次都要重新获取node的资源信息。

@DuodenumL DuodenumL force-pushed the feature/lock branch 2 times, most recently from 79da76e to 3f0e424 Compare March 3, 2022 02:20
type remapEntry struct {
ctx context.Context
nodeName string
logger log.Fields
Copy link
Contributor

Choose a reason for hiding this comment

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

这个还带个 logger 就很迷

// watch remap queue
func (c *Calcium) watchRemapChan() {
for {
if entry, ok := <-c.remapChan; ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

其实我的问题在于,你这样相当于是单机 remap,多 core 咋办

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我也发现这个问题了,但是还没来得及改…

应该是要新增一种类型的锁,专门给remap用

@DuodenumL DuodenumL changed the title remap without lock [WIP] remap without lock Mar 3, 2022
@jschwinger233
Copy link
Member

所以就按照最开始的讨论引入 node operation lock 了事

@DuodenumL DuodenumL changed the title [WIP] remap without lock add new types of node lock Mar 8, 2022
@DuodenumL
Copy link
Contributor Author

现在把node lock分成了两种,一种node resource lock,一种node operation lock(主要给remap用)。

@CMGS CMGS force-pushed the master branch 3 times, most recently from 06977aa to 837bec8 Compare March 9, 2022 04:21
nf := types.NodeFilter{
Includes: []string{nodename},
All: true,
}
return c.withNodesLocked(ctx, nf, func(ctx context.Context, nodes map[string]*types.Node) error {
return c.withNodesResourceLocked(ctx, nf, func(ctx context.Context, nodes map[string]*types.Node) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

这么多重复的代码,是我的话会抽出一个函数复用

Copy link
Contributor Author

Choose a reason for hiding this comment

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

改啦

@CMGS CMGS merged commit f1d9aa2 into projecteru2:master Mar 9, 2022
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

Successfully merging this pull request may close these issues.

3 participants