You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@brpc.apache.org by "chenBright (via GitHub)" <gi...@apache.org> on 2023/02/11 09:19:48 UTC

[GitHub] [brpc] chenBright opened a new pull request, #2123: Periodic ns quit

chenBright opened a new pull request, #2123:
URL: https://github.com/apache/brpc/pull/2123

   ### 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](../../master/CODE_OF_CONDUCT.md).(请遵循贡献者准则).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org

[GitHub] [brpc] wwbmmm commented on a diff in pull request #2123: Periodic ns quit

Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.

wwbmmm commented on code in PR #2123:
URL: https://github.com/apache/brpc/pull/2123#discussion_r1103970648


##########
src/brpc/periodic_naming_service.cpp:
##########
@@ -37,9 +37,13 @@ int PeriodicNamingService::RunNamingService(
     const char* service_name, NamingServiceActions* actions) {
     std::vector<ServerNode> servers;
     bool ever_reset = false;
-    for (;;) {
+    while (true) {
         servers.clear();
         const int rc = GetServers(service_name, &servers);
+        if (bthread_stopped(bthread_self())) {

Review Comment:
   这个放在57行的位置会不会更合适



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org

Re: [PR] Periodic ns quit (brpc)

Posted by "htner (via GitHub)" <gi...@apache.org>.

htner commented on PR #2123:
URL: https://github.com/apache/brpc/pull/2123#issuecomment-1694223680

    这个问题还是没有修复吗？


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org

Re: [PR] Periodic ns quit (brpc)

Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.

wwbmmm commented on PR #2123:
URL: https://github.com/apache/brpc/pull/2123#issuecomment-1436229573

   > @wwbmmm @serverglen 之前理解错了，ConsulNamingService其实不是PeriodicNamingService，不是简单的轮询，而是[长轮询](https://brpc.incubator.apache.org/zh/docs/client/basics/#consulservice-name)，每次rpc成功后，立即发起下一次长轮询。只有失败了，才会sleep。 [Blocking Queries](https://developer.hashicorp.com/consul/api-docs/features/blocking)：
   > 
   > > 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改回去？


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org