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

intro goroutine pool to speed up engine API call #343

Merged
merged 3 commits into from
Feb 22, 2021

Conversation

jschwinger233
Copy link
Member

No description provided.

@jschwinger233 jschwinger233 changed the title [WIP] intro goroutine pool to speed up engine API call intro goroutine pool to speed up engine API call Feb 18, 2021
@CMGS
Copy link
Contributor

CMGS commented Feb 18, 2021

  1. lint failed
  2. core.yaml.sample 增加这个参数

@jschwinger233 jschwinger233 force-pushed the zc/conn branch 2 times, most recently from d78461c to e6587e4 Compare February 18, 2021 09:54
@jschwinger233 jschwinger233 changed the title intro goroutine pool to speed up engine API call [WIP] intro goroutine pool to speed up engine API call Feb 18, 2021
@jschwinger233 jschwinger233 changed the title [WIP] intro goroutine pool to speed up engine API call intro goroutine pool to speed up engine API call Feb 18, 2021
@CMGS
Copy link
Contributor

CMGS commented Feb 19, 2021

github.com/projecteru2/core/discovery/helium 这个部分改了啥,为啥覆盖率下降了4个点

@CMGS
Copy link
Contributor

CMGS commented Feb 19, 2021

github.com/projecteru2/core/discovery/helium 这个部分改了啥,为啥覆盖率下降了4个点

看来是鸡蛋引入的

Comment on lines +210 to +214
if e := sem.Acquire(context.Background(), c.config.MaxConcurrency); e != nil {
log.Errorf("[Calcium.doDeployWorkloadsOnNode] Failed to wait all workers done: %+v", e)
err = e
indices = utils.Range(deploy)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

如果这时候对 core C+C 会有什么结果

Copy link
Member Author

Choose a reason for hiding this comment

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

理论上由于使用了 Background ctx, 这里不会有 error;
如果阻塞在 Acquire(MaxConn), 那 c-c 也会继续阻塞在这里, 但是分发下去的 goroutine 去调用 docker api 的时候就会被 cancel, 最终导致部分创建容器成功部分失败, 然后在那些 goroutine defer 里 release 之后, Acquire(MaxConn) 成功 return.

Copy link
Member Author

Choose a reason for hiding this comment

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

其实就和 WaitGroup.Wait() 是一样的了, wg.Wait() 直接就不让 ctx 打断, 这里使用 Background 就回到 wg.Wait()

@jschwinger233
Copy link
Member Author

github.com/projecteru2/core/discovery/helium 这个部分改了啥,为啥覆盖率下降了4个点

本地运行是 88%, CI 就变成 82% 了...

@@ -191,6 +191,7 @@ func processVirtualizationOutStream(
if split != 0 {
bs = append(bs, split)
}
println(string(bs))
Copy link
Contributor

Choose a reason for hiding this comment

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

println 干掉

Comment on lines +73 to +78
networks := map[string]string{}
for name, network := range opts.Networks {
networkMode = dockercontainer.NetworkMode(name)
networks[name] = network
if networkMode.IsHost() {
opts.Networks[name] = ""
networks[name] = ""
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
Member Author

Choose a reason for hiding this comment

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

新创建了局部变量 map, 而不是修改传入的指针, 因为这里变成并发调用了, 并发修改指针 map 不可以.

@CMGS
Copy link
Contributor

CMGS commented Feb 19, 2021

github.com/projecteru2/core/discovery/helium 这个部分改了啥,为啥覆盖率下降了4个点

本地运行是 88%, CI 就变成 82% 了...

我本地运行还惨一些,直接 segment fault

@CMGS CMGS merged commit 7a3f6f5 into projecteru2:master Feb 22, 2021
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.

2 participants