-
Notifications
You must be signed in to change notification settings - Fork 42
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
optimize engine cache checker #539
optimize engine cache checker #539
Conversation
}() | ||
|
||
keysToCheck.Range(func(key, _ interface{}) bool { | ||
pool.Go(ctx, func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sync.Map. Range 里用 goroutine 好像会导致并发不安全, 本来有互斥锁, 但是 goroutine 逃逸了这个锁.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
改了,不知道为啥这里没标outdated
aa94e3e
to
7e17f11
Compare
} | ||
|
||
// ImageRemoteDigest . | ||
func (f *Engine) ImageRemoteDigest(ctx context.Context, image string) (string, error) { | ||
return "", types.ErrNilEngine | ||
return "", f.DefaultErr | ||
} | ||
|
||
// ImageBuildFromExist . | ||
func (f *Engine) ImageBuildFromExist(ctx context.Context, ID string, refs []string, user string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmmm 有点过分了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里是为了让error进入缓存,比如创建Engine的时候错误是connection refused,那么就让这个fake engine每次调用都返回connection refused
engine/factory/factory.go
Outdated
} | ||
if err := validateEngine(ctx, client, config.ConnectionTimeout); err != nil { | ||
log.Errorf(ctx, "[EngineCacheChecker] engine %v is unavailable, will be replaced with a fake engine, err: %v", cacheKey, err) | ||
engineCache.Set(cacheKey, &fake.Engine{DefaultErr: err}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里更新 cache 用了两步操作, 先 delete 再 set, 可以合并成一步吗, 合并成一步是不是就并发安全了
engine/factory/factory.go
Outdated
// EngineCacheChecker checks if the engine in cache is available | ||
func EngineCacheChecker(ctx context.Context, timeout time.Duration) { | ||
func EngineCacheChecker(ctx context.Context, config types.Config) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里已经应该建模了吧, 不要再操作一个全局变量了, 创建一个结构体, 包含 cache, 然后这个函数作为其中一个方法.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
改好了
44dd656
to
b60db40
Compare
engine/factory/factory.go
Outdated
}() | ||
|
||
// getEngine get engine | ||
func getEngine(ctx context.Context, config types.Config, nodename, endpoint, ca, cert, key string) (client engine.API, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个名字应该是 newEngine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好像还行 自己要跑一跑试一下
b60db40
to
472f41c
Compare
跑了一下,截至目前没发现问题 |
优化了一下Engine Cache Check的逻辑,即使GetEngine报错了也要记录到缓存里,否则在endpoint三层不通的时候,每次ping都要花很长时间,导致一些api例如GetWorkload耗时巨大。