[ISSUE #9308]For the orchestration of configuration filters, there are the following areas that can be optimized #9309 by CLFutureX · Pull Request #15034 · alibaba/nacos

CLFutureX · 2026-05-06T10:27:18Z

This PR replaces the previous one. See below for detailed changes. old pr

# Conflicts: # client/src/main/java/com/alibaba/nacos/client/config/filter/impl/ConfigFilterChainManager.java # client/src/test/java/com/alibaba/nacos/client/config/filter/impl/ConfigFilterChainManagerTest.java

github-actions · 2026-05-06T10:28:28Z

Thanks for your this PR. 🙏
Please check again for your PR changes whether contains any usage/api/configuration change such as Add new API , Add new configuration, Change default value of configuration.
If so, please add or update documents(markdown type) in docs/next/ for repository nacos-group/nacos-group.github.io

感谢您提交的PR。 🙏
请再次查看您的PR内容，确认是否包含任何使用方式/API/配置参数的变更，如：新增API、新增配置参数、修改默认配置等操作。
如果是，请确保在提交之前，在仓库nacos-group/nacos-group.github.io中的docs/next/目录下添加或更新文档（markdown格式）。

KomachiSion · 2026-05-07T02:36:31Z

-        
+
        IConfigContext configContext = configRequest.getConfigContext();
-        


Please don't change the indent

KomachiSion · 2026-05-09T07:29:04Z

@CLFutureX So sorry, nacos currently is doing spotless auto reformat code works so that your pr is conflict with this work.

Please solve conflict and use mvn spotless:apply after you solve conflicts

codecov-commenter · 2026-05-09T07:44:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

KomachiSion · 2026-05-26T06:48:20Z

@CLFutureX Sorry, there is one file is in conflicts. Please help to resolve it.

nacos-community · 2026-08-25T09:36:14Z

⚠️ Merge conflict detected

This PR has conflicts with the develop branch and cannot be merged in its current state. Please rebase or merge develop into your branch and resolve the conflicts:

git fetch origin
git checkout RapperCL_fix_9308_new
git rebase origin/develop
# resolve conflicts, then:
git push --force-with-lease

This is a one-time reminder. Feel free to @mention me for a re-review after conflicts are resolved.

Automated notification by github-manager-bot

nacos-community

This PR refactors the client config filter chain so doFilter() no longer allocates a new VirtualFilterChain per invocation, using a prebuilt immutable linked chain instead. The direction is good and execution order is preserved, but the shared filterChain field lacks a visibility guarantee for reader threads — please address the critical finding below.

Findings

[Critical] client/src/main/java/com/alibaba/nacos/client/config/filter/impl/ConfigFilterChainManager.java:41 — filterChain should be volatile to prevent stale/null reads from unsynchronized doFilter().
[Warning] ConfigFilterChainManager.java:55 — buildConfigFilterChain() should be private, not public.
[Info] ConfigFilterChainManager.java:49 — redundant final buildConfigFilterChain() call in the constructor.
[Info] ConfigFilterChainManager.java:92 — chain rebuilt on every addFilter(), making bulk init O(n²).
[Info] client/src/test/java/com/alibaba/nacos/client/config/filter/impl/ConfigFilterChainTest.java:30 — no coverage for the new structure or concurrency.

Suggestions

private volatile IConfigFilterChain filterChain; (or hold the chain in a final holder published safely) is the minimal fix.
Consider rebuilding the chain only once after the initial ServiceLoader batch, and making buildConfigFilterChain() private.

Automated review by github-manager-bot

nacos-community · 2026-09-03T13:43:43Z

    private final Properties initProperty;
-
+
+    private IConfigFilterChain filterChain;


filterChain is a non-volatile field written in the constructor / synchronized addFilter() and read from unsynchronized doFilter(). Client config filters run on listener/worker threads, so readers may observe a stale or null chain. Change to private volatile IConfigFilterChain filterChain; to ensure safe publication on every rebuild.

nacos-community · 2026-09-03T13:43:43Z

+    /**
+     * Build ConfigFilterChain.
+     */
+    public void buildConfigFilterChain() {


buildConfigFilterChain() is declared public but is only an internal implementation detail used by this class. Reduce API surface by making it private to avoid exposing new public methods in the client module.

nacos-community · 2026-09-03T13:43:43Z

        for (IConfigFilter configFilter : configFilters) {
            addFilter(configFilter);
        }
+        buildConfigFilterChain();


The constructor already triggers buildConfigFilterChain() inside each addFilter() call, so this final call is redundant. Consider building the chain only once after all initial filters are added.

nacos-community · 2026-09-03T13:43:43Z

            this.filters.add(i, filter);
        }
+        buildConfigFilterChain();
        return this;


addFilter() rebuilds the entire chain from scratch on every insertion, making bulk initialization (e.g., ServiceLoader in the constructor) O(n²). For typical filter counts this is fine, but consider whether the chain should be rebuilt only once after batch additions.

nacos-community · 2026-09-03T13:43:43Z

@@ -28,6 +28,7 @@ void testConfigFilterChain() {
        ConfigFilterChainManager configFilterChainManager = new ConfigFilterChainManager(null);
        configFilterChainManager.addFilter(new DemoFilter1());
        configFilterChainManager.addFilter(new DemoFilter2());


The test change is only a blank line. Existing coverage preserves ordering with two filters, but does not exercise the new linked-node structure or guard against the visibility issue. Consider adding a multi-thread doFilter test or a test that verifies the chain is not recreated per invocation.

CLFutureX and others added 10 commits October 11, 2022 20:56

ConfigFilterChain optimization

5ef5a83

Merge branch 'develop' into RapperCL_fix_9308

ba1b894

name modify

4a3b76f

Format to optimize

100943f

add buildConfigFilterChain

78aafc9

Format optimization and add buildConfigFilterChain

3c6b4af

Format optimization

3918476

Format optimization

c8c7f65

Merge branch 'develop' into RapperCL_fix_9308

9ccdde7

# Conflicts: # client/src/main/java/com/alibaba/nacos/client/config/filter/impl/ConfigFilterChainManager.java # client/src/test/java/com/alibaba/nacos/client/config/filter/impl/ConfigFilterChainManagerTest.java

pr update and logic optimization

afc35b9

KomachiSion requested changes May 7, 2026

View reviewed changes

CLFutureX added 4 commits May 7, 2026 14:43

update

08e490a

update

8e57cc3

update

f2c798d

update

dbb22a1

CLFutureX requested a review from KomachiSion May 7, 2026 07:21

KomachiSion previously approved these changes May 7, 2026

View reviewed changes

KomachiSion added the kind/code quality label May 7, 2026

code style update

5365b7b

CLFutureX dismissed KomachiSion’s stale review via 5365b7b May 8, 2026 03:00

nacos-community suggested changes Sep 3, 2026

View reviewed changes

Travis696 approved these changes Sep 4, 2026

View reviewed changes

Navigation Menu

Uh oh!

[ISSUE #9308]For the orchestration of configuration filters, there are the following areas that can be optimized #9309 - #15034

[ISSUE #9308]For the orchestration of configuration filters, there are the following areas that can be optimized #9309#15034
CLFutureX wants to merge 15 commits into
alibaba:developfrom
CLFutureX:RapperCL_fix_9308_new

CLFutureX commented May 6, 2026

Uh oh!

github-actions Bot commented May 6, 2026

Uh oh!

KomachiSion May 7, 2026

Uh oh!

KomachiSion commented May 9, 2026

Uh oh!

codecov-commenter commented May 9, 2026

Uh oh!

KomachiSion commented May 26, 2026

Uh oh!

nacos-community commented Aug 25, 2026

Uh oh!

nacos-community left a comment

Uh oh!

nacos-community Sep 3, 2026

Uh oh!

nacos-community Sep 3, 2026

Uh oh!

nacos-community Sep 3, 2026

Uh oh!

nacos-community Sep 3, 2026

Uh oh!

nacos-community Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants



		IConfigContext configContext = configRequest.getConfigContext();

		private final Properties initProperty;


		private IConfigFilterChain filterChain;

Uh oh!

Conversation

CLFutureX commented May 6, 2026

Uh oh!

github-actions Bot commented May 6, 2026

Uh oh!

KomachiSion May 7, 2026

Choose a reason for hiding this comment

Uh oh!

KomachiSion commented May 9, 2026

Uh oh!

codecov-commenter commented May 9, 2026

Codecov Report

Uh oh!

KomachiSion commented May 26, 2026

Uh oh!

nacos-community commented Aug 25, 2026

Uh oh!

nacos-community left a comment

Choose a reason for hiding this comment

Findings

Suggestions

Uh oh!

nacos-community Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

nacos-community Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

nacos-community Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

nacos-community Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

nacos-community Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants