-
Notifications
You must be signed in to change notification settings - Fork 4k
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 gflags bthread_concurrency_by_tag validate error #2730
Conversation
一般什么情况下会两次解析呀,我这边确实没遇到过这样的场景,这块就做的不充分。 |
两次解析是由于上层应用对其他的gflags有reload需求,但使用了类似ParseCommandLineFlags会调用所有flags的validate函数?或者只是代码的不规范导致的在链路中执行了两次ParseCommandLineFlags触发这个问题。这部分应该是可以通过修改上层应用的代码来避免,但是brpc应该也需要做到兼容这个场景。
如果内部实现有多个tag,一般来说第一次解析是在main函数开始出,此时第一次解析由于设置了never_set_bthread_concurrency_by_tag,task_control还是等于nullptr;后续如果在设置了tag的server没启动前出现第二次解析,此时task_control init之后只有tag 0,因此指定其他的tag时,tag_ngroup = 0,如果设置了bthread_concurrency_by_tag的值,那么只要其大于0,那么就会触发add_workers,不会有异常。如果在server启动之后第二次触发,就和正常的动态改变分组线程的流程一致了,应该也没有问题。 |
我觉得得测试一下这些场景,如果设置了task_group_ntags=2,那么在task_control init的时候会把FLAGS_bthread_concurrency平均分配给每组。这时候再执行bthread_setconcurrency_by_tag把tag0的worker数量改变了,这个线程数量是你想要的吗?感觉这块考虑的是不是过于复杂了。 |
是的,默认设置为BTHREAD_TAG_INVALID更合理,按照你的意见修改了 |
91ad057
to
66dbcb0
Compare
df9fbc8
to
a47fdd5
Compare
LGTM |
1 similar comment
LGTM |
What problem does this PR solve?
Issue Number:
#2542
Problem Summary:
升级到1.10.0版本后,重复解析两次 gflags 后,flags bthread_concurrency_by_tag validate会报错的问题又复现了。
是由于在默认配置下,第二次解析gflags时,num = 0,tag_ngroup = FALGS_bthread_concurrency。两者不想等导致返回了EPERM。
gflags的默认配置下,只有一个tag且等于0,bthread_concurrency_by_tag的默认值应该设置成等于bthread_concurrency的默认值,这样也比较合理。
What is changed and the side effects?
Changed:
Side effects:
Performance effects(性能影响):
Breaking backward compatibility(向后兼容性):
Check List: