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

fix(registry/consul): fix concurrency issues and improve performance #3511

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lftk
Copy link
Contributor

@lftk lftk commented Jan 2, 2025

修复了一些并发相关的问题:

  1. Registry 锁的范围太大,比如 Watch 对整个函数加锁,不能同时处理多个(不同的)Service;

func (r *Registry) Watch(ctx context.Context, name string) (registry.Watcher, error) {
r.lock.Lock()
defer r.lock.Unlock()

  1. 相同的 Service 没有共享 resolve,只要 Watch 就会新开一个 resolve 协程;

if err := r.resolve(ctx, set); err != nil {
return nil, err
}

  1. Client 注销任一 Service 都会直接 cancelClient.ctx,导致所有的 heartbeat 协程退出;

func (c *Client) Deregister(_ context.Context, serviceID string) error {
defer c.cancel()
return c.cli.Agent().ServiceDeregister(serviceID)
}

  1. Watch 时,由于 w 先被添加到 set 中,然后才会写入 services event,如果在两步中间正好遇到之前的 resolvebroadcast,209行的写入则会被卡住(w 还没有返回,不会有调用者主动调用 Next 消费)。

// init watcher
w := &watcher{
event: make(chan struct{}, 1),
}
w.ctx, w.cancel = context.WithCancel(ctx)
w.set = set
set.lock.Lock()
set.watcher[w] = struct{}{}
set.lock.Unlock()
ss, _ := set.services.Load().([]*registry.ServiceInstance)
if len(ss) > 0 {
// If the service has a value, it needs to be pushed to the watcher,
// otherwise the initial data may be blocked forever during the watch.
w.event <- struct{}{}
}
if err := r.resolve(ctx, set); err != nil {
return nil, err
}
return w, nil

  1. 其他,提前判断 ctx.Err() 避免做冗长的无用功、sleepCtx 等。

@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant