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:#1143 Feature/reduce etcd registry conn; wait group modify #1297

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
7 changes: 6 additions & 1 deletion registry/etcdv3/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ func newETCDV3Registry(url *common.URL) (registry.Registry, error) {
return nil, err
}

go etcdv3.HandleClientRestart(r)

r.handleClientRestart()
r.InitListeners()

return r, nil
Expand Down Expand Up @@ -174,3 +174,8 @@ func (r *etcdV3Registry) DoSubscribe(svc *common.URL) (registry.Listener, error)
func (r *etcdV3Registry) DoUnsubscribe(conf *common.URL) (registry.Listener, error) {
return nil, perrors.New("DoUnsubscribe is not support in etcdV3Registry")
}

func (r *etcdV3Registry) handleClientRestart() {
Copy link
Contributor

Choose a reason for hiding this comment

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

AsyncHandleClientRestart

r.WaitGroup().Add(1)
go etcdv3.HandleClientRestart(r)
}
6 changes: 6 additions & 0 deletions registry/zookeeper/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ func newZkRegistry(url *common.URL) (registry.Registry, error) {
return nil, err
}

r.WaitGroup().Add(1)
go zookeeper.HandleClientRestart(r)

r.listener = zookeeper.NewZkEventListener(r.client)
Expand Down Expand Up @@ -312,3 +313,8 @@ func (r *zkRegistry) getCloseListener(conf *common.URL) (*RegistryConfigurationL

return zkListener, nil
}

func (r *zkRegistry) handleClientRestart() {
Copy link
Contributor

Choose a reason for hiding this comment

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

AsyncHandleClientRestart

r.WaitGroup().Add(1)
go zookeeper.HandleClientRestart(r)
}
2 changes: 1 addition & 1 deletion remoting/etcdv3/facade.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ type clientFacade interface {
}

// HandleClientRestart keeps the connection between client and server
// This method should be used only once. You can use handleClientRestart() in package registry.
func HandleClientRestart(r clientFacade) {
r.WaitGroup().Add(1)
defer r.WaitGroup().Done()
Copy link
Contributor

Choose a reason for hiding this comment

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

r.WaitGroup()的done,wait和add使用散落在各处,remoting作为基础组件,是否可能有同学直接调用 HandleClientRestart ,然后函数结束出现 negative 的问题?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

有可能,用once限制一下HandleClientRestart的调用次数是否可以呢;该方法在init包被引入的时候调用,只调用一次。

for {
select {
Expand Down
2 changes: 1 addition & 1 deletion remoting/zookeeper/facade.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ type ZkClientFacade interface {
}

// HandleClientRestart keeps the connection between client and server
// This method should be used only once. You can use handleClientRestart() in package registry.
func HandleClientRestart(r ZkClientFacade) {
r.WaitGroup().Add(1)
defer r.WaitGroup().Done()
for {
select {
Expand Down