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

[ISSUE #1122] support custom asyncThreadPool #1194

Closed
wants to merge 2 commits into from

Conversation

loongs-zhang
Copy link

see #1122

@sofastack-bot
Copy link

sofastack-bot bot commented Apr 16, 2022

Hi @dragon-zhang, welcome to SOFAStack community, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

@sofastack-bot sofastack-bot bot added cla:no Need sign CLA First-time contributor First-time contributor size/XS labels Apr 16, 2022
@loongs-zhang
Copy link
Author

111111企业微信截图_2e680565-6575-4875-ab2a-52efaa9e94bc

我明明签了CLA呀

@seeflood seeflood closed this Apr 16, 2022
@seeflood seeflood reopened this Apr 16, 2022
@sofastack-bot sofastack-bot bot added the cla:yes CLA is ok label Apr 16, 2022
@seeflood
Copy link
Member

@dragon-zhang cla好啦,感谢贡献。

@loongs-zhang
Copy link
Author

@dragon-zhang cla好啦,感谢贡献。

em...master的测试用例有些问题,这个PR只加了设置线程池的方法.

@codecov
Copy link

codecov bot commented Apr 17, 2022

Codecov Report

Merging #1194 (b456939) into master (2f23db9) will decrease coverage by 0.07%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #1194      +/-   ##
============================================
- Coverage     71.58%   71.50%   -0.08%     
+ Complexity      830      827       -3     
============================================
  Files           408      408              
  Lines         17211    17213       +2     
  Branches       2682     2682              
============================================
- Hits          12320    12309      -11     
- Misses         3529     3540      +11     
- Partials       1362     1364       +2     
Impacted Files Coverage Δ
...java/com/alipay/sofa/rpc/context/AsyncRuntime.java 78.57% <0.00%> (-6.05%) ⬇️
...m/alipay/sofa/rpc/module/FaultToleranceModule.java 63.63% <0.00%> (-31.82%) ⬇️
...ofa/rpc/registry/consul/HealthServiceInformer.java 84.78% <0.00%> (-4.35%) ⬇️
.../main/java/com/alipay/sofa/rpc/event/EventBus.java 78.43% <0.00%> (-1.97%) ⬇️
.../alipay/sofa/rpc/metrics/lookout/RpcLookoutId.java 87.30% <0.00%> (-1.59%) ⬇️
...om/alipay/sofa/rpc/metrics/lookout/RpcLookout.java 80.00% <0.00%> (-0.96%) ⬇️
...n/java/com/alipay/sofa/rpc/common/SofaConfigs.java 86.79% <0.00%> (+1.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f23db9...b456939. Read the comment docs.

* @param asyncThreadPool callback用的线程池
*/
public static void setAsyncThreadPool(ThreadPoolExecutor asyncThreadPool) {
AsyncRuntime.asyncThreadPool = asyncThreadPool;
Copy link
Member

Choose a reason for hiding this comment

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

We need to consider a more reasonable configuration for supporting asynchronous thread pool setups. The changes here do not guarantee thread safety and do not ensure that no other logic will have access to the thread pool created by the original logic before the set method is called. I hope to have a modified solution to discuss before putting it into development.

Copy link
Author

Choose a reason for hiding this comment

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

We need to consider a more reasonable configuration for supporting asynchronous thread pool setups. The changes here do not guarantee thread safety and do not ensure that no other logic will have access to the thread pool created by the original logic before the set method is called. I hope to have a modified solution to discuss before putting it into development.

sorry, I will follow the steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:no Need sign CLA cla:yes CLA is ok First-time contributor First-time contributor size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants