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

Periodic ns quit #2123

Merged
merged 3 commits into from
Feb 15, 2023
Merged

Periodic ns quit #2123

merged 3 commits into from
Feb 15, 2023

Conversation

chenBright
Copy link
Contributor

@chenBright chenBright commented Feb 11, 2023

What problem does this PR solve?

Issue Number: #584

Problem Summary: 继承自PeriodicNamingService的ns,例如DiscoveryNamingService、NacosNamingService、RemoteFileNamingService,在GetServers中通过rpc拉取节点的时候,NamingServiceThread析构调用bthread_stop并不能中断CallMethod,CallMethod只是被唤醒了一下,而且将interrupted置回为false,CallMethod依然会等到rpc结束。但是这时候bthread_usleep已经不能感知到bthread_interrupt而退出了。

What is changed and the side effects?

  1. PeriodicNamingService通过判断bthread_stopped来退出。
  2. ConsulNamingService本质上是一个PeriodicNamingService,将其抽象为PeriodicNamingService的派生类。

Changed:

Side effects:

  • Performance effects(性能影响):

  • Breaking backward compatibility(向后兼容性):


Check List:

  • Please make sure your changes are compilable(请确保你的更改可以通过编译).
  • When providing us with a new feature, it is best to add related tests(如果你向我们增加一个新的功能, 请添加相关测试).
  • Please follow Contributor Covenant Code of Conduct.(请遵循贡献者准则).

@wwbmmm
Copy link
Contributor

wwbmmm commented Feb 14, 2023

LGTM

1 similar comment
@serverglen
Copy link
Contributor

LGTM

@serverglen serverglen merged commit 3d4db04 into apache:master Feb 15, 2023
@chenBright chenBright deleted the periodic_ns_quit branch February 15, 2023 02:20
@chenBright
Copy link
Contributor Author

@wwbmmm @serverglen
之前理解错了,ConsulNamingService其实不是PeriodicNamingService,不是简单的轮询,而是长轮询,每次rpc成功后,立即发起下一次长轮询。只有失败了,才会sleep。
Blocking Queries

Many endpoints in Consul support a feature known as "blocking queries". A blocking query is used to wait for a potential change using long polling.

这个pr可能需要回退一下或者重新提一个pr将ConsulNamingService改回去。

@wwbmmm
Copy link
Contributor

wwbmmm commented Feb 20, 2023

@wwbmmm @serverglen 之前理解错了,ConsulNamingService其实不是PeriodicNamingService,不是简单的轮询,而是长轮询,每次rpc成功后,立即发起下一次长轮询。只有失败了,才会sleep。 Blocking Queries

Many endpoints in Consul support a feature known as "blocking queries". A blocking query is used to wait for a potential change using long polling.

这个pr可能需要回退一下或者重新提一个pr将ConsulNamingService改回去。

要不重新提一个pr把ConsulNamingService改回去?

@chenBright
Copy link
Contributor Author

@wwbmmm @serverglen 之前理解错了,ConsulNamingService其实不是PeriodicNamingService,不是简单的轮询,而是长轮询,每次rpc成功后,立即发起下一次长轮询。只有失败了,才会sleep。 Blocking Queries

Many endpoints in Consul support a feature known as "blocking queries". A blocking query is used to wait for a potential change using long polling.

这个pr可能需要回退一下或者重新提一个pr将ConsulNamingService改回去。

要不重新提一个pr把ConsulNamingService改回去?

好的,我今天改一下。

yanglimingcn pushed a commit to yanglimingcn/brpc that referenced this pull request Jun 25, 2023
* Fix periodic ns thread can not quit

* Abstract ConsulNamingService into a PeriodicNamingService

* Quit periodic ns when bthread is stopped
yanglimingcn pushed a commit to yanglimingcn/brpc that referenced this pull request Oct 31, 2023
* Fix periodic ns thread can not quit

* Abstract ConsulNamingService into a PeriodicNamingService

* Quit periodic ns when bthread is stopped
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.

4 participants