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 #9308]For the orchestration of configuration filters, there are the following areas that can be optimized #9309 #9310

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

CLFutureX
Copy link
Contributor

@CLFutureX CLFutureX commented Oct 12, 2022

1 Add FilterChainNode as the responsibility chain node to package the configuration filter, so each node contains the current configuration filter and the next responsibility chain node.
2 At startup, build a chain of responsibility through the buildConfigFilterChain() method.
3 Create EmptyFilterChainNode as the last node on the chain to reduce the next == null judgment during execution

1 新增FilterChainNode作为责任链节点,对配置过滤器进行包装,于是每个节点包含当前的配置过滤器,以及下一个责任链节点。
2 在启动时,通过buildConfigFilterChain()方法构建责任链。
3 创建EmptyFilterChainNode作为链上的最后一个节点,减少在执行过程中的 next == null 判断

issue

@CLFutureX CLFutureX changed the title [ISSUE 9308]For the orchestration of configuration filters, there are the following areas that can be optimized #9309 [ISSUE #9308]For the orchestration of configuration filters, there are the following areas that can be optimized #9309 Oct 12, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #9310 (c8c7f65) into develop (8aa2e0a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #9310      +/-   ##
=============================================
+ Coverage      44.10%   44.12%   +0.01%     
- Complexity      4862     4866       +4     
=============================================
  Files            913      913              
  Lines          33052    33057       +5     
  Branches        3828     3828              
=============================================
+ Hits           14579    14587       +8     
- Misses         17052    17053       +1     
+ Partials        1421     1417       -4     
Impacted Files Coverage Δ
...t/config/filter/impl/ConfigFilterChainManager.java 97.29% <100.00%> (+0.42%) ⬆️
...m/alibaba/nacos/naming/misc/HttpClientManager.java 69.04% <0.00%> (-14.29%) ⬇️
...cos/core/cluster/remote/ClusterRpcClientProxy.java 72.85% <0.00%> (-1.43%) ⬇️
...baba/nacos/naming/healthcheck/HealthCheckTask.java 45.58% <0.00%> (ø)
...alibaba/nacos/client/config/impl/ClientWorker.java 55.36% <0.00%> (+0.19%) ⬆️
.../core/v2/event/publisher/NamingEventPublisher.java 76.25% <0.00%> (+1.25%) ⬆️
...a/nacos/core/remote/control/TpsMonitorManager.java 54.77% <0.00%> (+1.27%) ⬆️
...alibaba/nacos/core/remote/control/TpsRecorder.java 75.67% <0.00%> (+1.35%) ⬆️
...m/alibaba/nacos/core/remote/ConnectionManager.java 44.37% <0.00%> (+1.47%) ⬆️

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 8aa2e0a...c8c7f65. Read the comment docs.

@KomachiSion
Copy link
Collaborator

KomachiSion commented Oct 12, 2022

Can you discuss with communities first?

@KomachiSion KomachiSion added the pending On hold due to dependency or release label Oct 12, 2022
@CLFutureX
Copy link
Contributor Author

Can you discuss with communities first?

Yes, I have already raised the corresponding issue, welcome to discuss together

@HLAINGMYOOO
Copy link

good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending On hold due to dependency or release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants