[ISSUE #15470] Upgrade SOFA-JRaft to 1.4.1

[jay666mnj]

Please do not create a Pull Request without creating an issue first.

What is the purpose of the change

Upgrade SOFA-JRaft dependencies to 1.4.1 as part of the Nacos 3.3.0 dependency upgrade tracking in #15470.

Brief change

• Upgrade jraft-core from 1.4.0 to 1.4.1.
• Keep rpc-grpc-impl aligned through ${jraft-core.version}.
• Replace the removed transitive com.alipay.hessian.clhm.ConcurrentLinkedHashMap usage in RpcAckCallbackSynchronizer with JDK concurrent map handling.

Verifying this change

• mvn -pl core spotless:check -DskipTests
• mvn -pl core -am -Dtest=RpcAckCallbackSynchronizerTest -Dsurefire.failIfNoSpecifiedTests=false test
• mvn -pl core -am -DskipTests compile
• mvn -pl core dependency:tree -Dincludes=com.alipay.sofa,com.caucho:hessian -DskipTests

Follow this checklist to help us incorporate your contribution quickly and easily:

• Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
• Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
• Run mvn -B clean package apache-rat:check spotbugs:check -DskipTests to make sure basic checks pass. Run mvn clean install to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

[github-actions]

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]

Request changes: preserve bounded callback behavior without silently losing ACK futures

Replacing the transitive com.alipay.sofa:hessian usage is reasonable, but the current manual trimming implementation is not concurrency-equivalent to the original ConcurrentLinkedHashMap.

There is a race between context initialization, future registration, and trimming:

1. initContextIfNecessary() inserts and returns a context.
2. trimCallbackContextIfNecessary() removes that context from CALLBACK_CONTEXT.
3. syncCallback() still holds the removed map and inserts a future into it.
4. The ACK path later looks up the connection through CALLBACK_CONTEXT and cannot find the detached context.
5. The future is not completed by the trim operation and eventually appears as an ordinary request timeout.

This is misleading during incident investigation because the client may have returned the ACK normally, while the server discarded the matching context because of capacity pressure.

Please consider one of the following two options.

---

Option 1 — Keep the JDK-only replacement and make trimming concurrency-safe

This is the preferred option because it keeps the change local to RpcAckCallbackSynchronizer, introduces no dependency, and avoids maintaining a complete third-party concurrent cache implementation.

1. Preserve the public field type

CALLBACK_CONTEXT is public and currently declared as Map. Changing the declared field type to ConcurrentMap may introduce binary compatibility concerns.

Please keep the public declaration and back it with a private concurrent store:

private static final ConcurrentMap<String, Map<String, DefaultRequestFuture>>
        CALLBACK_CONTEXT_STORE = new ConcurrentHashMap<>(128);

public static final Map<String, Map<String, DefaultRequestFuture>> CALLBACK_CONTEXT =
        CALLBACK_CONTEXT_STORE;

2. Use ConcurrentHashMap for the inner map

The per-connection map is concurrently accessed by request, ACK and timeout threads. Retaining HashMap is unsafe.

Map<String, DefaultRequestFuture> context =
        new ConcurrentHashMap<>(128);

3. Verify context ownership after future registration

After inserting a future, verify that the context is still the current value associated with the connection ID. If it was removed by trimming, withdraw the future from the detached context and retry.

while (true) {
    Map<String, DefaultRequestFuture> context =
        initContextIfNecessary(connectionId);
    DefaultRequestFuture previous = context.putIfAbsent(requestId, future);

    if (CALLBACK_CONTEXT_STORE.get(connectionId) == context) {
        if (previous == null) {
            return;
        }
        throw new NacosException(
            NacosException.INVALID_PARAM, "request id conflict");
    }

    if (previous == null && !context.remove(requestId, future)) {
        // ACK, timeout, or trim already owns completion of this future.
        return;
    }

    // The future was removed from a detached context; retry registration.
}

Normally this loop executes once. A retry is only needed when registration races with trimming near the capacity limit.

4. Claim each future before completing it

After removing a context from the outer map, trim must use conditional removal to obtain ownership of each future:

if (removed.remove(requestId, future)) {
    future.setFailResult(new TimeoutException(...));
}

This gives deterministic ownership:

• if ACK removes the future first, ACK completes it;
• if trim removes the future first, trim completes it with the capacity error;
• timeout and clearFuture follow the same ownership rule;
• the future cannot be completed twice by ACK and trim.

Please catch failures independently for each future. One callback throwing an exception must not stop cleanup of the remaining futures or weaken the capacity bound.

private static int failRemovedContext(
        String connectionId,
        Map<String, DefaultRequestFuture> removed) {
    if (removed == null) {
        return 0;
    }

    int failedCount = 0;
    for (Map.Entry<String, DefaultRequestFuture> entry : removed.entrySet()) {
        String requestId = entry.getKey();
        DefaultRequestFuture future = entry.getValue();
        if (removed.remove(requestId, future)) {
            failedCount++;
            try {
                future.setFailResult(new TimeoutException(
                    "RPC ACK future was evicted because callback context capacity "
                        + "was exceeded, connectionId=" + connectionId
                        + ", requestId=" + requestId
                        + ", maxContextSize=" + MAX_CALLBACK_CONTEXT_SIZE));
            } catch (Throwable throwable) {
                Loggers.REMOTE_DIGEST.warn(
                    "Failed to notify an evicted RPC ACK future, "
                        + "connectionId={}, requestId={}",
                    connectionId, requestId, throwable);
            }
        }
    }
    return failedCount;
}

5. Keep the hard capacity bound

The outer map can still use simple unordered eviction:

private static void trimCallbackContextIfNecessary() {
    while (CALLBACK_CONTEXT_STORE.size() > MAX_CALLBACK_CONTEXT_SIZE) {
        Iterator<String> iterator = CALLBACK_CONTEXT_STORE.keySet().iterator();
        if (!iterator.hasNext()) {
            return;
        }

        int sizeBefore = CALLBACK_CONTEXT_STORE.size();
        String connectionId = iterator.next();
        Map<String, DefaultRequestFuture> removed =
            CALLBACK_CONTEXT_STORE.remove(connectionId);

        int failedFutureCount = failRemovedContext(connectionId, removed);
        logCapacityTrimIfNecessary(
            connectionId, failedFutureCount, sizeBefore,
            CALLBACK_CONTEXT_STORE.size());
    }
}

This does not preserve exact LRU ordering, but it preserves the important bounded-capacity and eviction-notification behavior without silently detaching futures.

Strict concurrent LRU cannot reasonably be reproduced with a small JDK-only implementation while also keeping the implementation simple and avoiding additional dependencies.

6. Add explicit capacity-trim diagnostics

Please do not complete evicted futures with an empty TimeoutException. A capacity-triggered eviction is not an ordinary remote ACK timeout and must be distinguishable during troubleshooting.

When a trimmed context contains pending futures, emit a WARN log containing:

• a stable reason such as RPC_ACK_CALLBACK_CONTEXT_CAPACITY_EXCEEDED;
• the evicted connection ID;
• the number of pending futures claimed by trim;
• context size before and after trimming;
• the configured maximum context size.

For example:

RPC_ACK_CALLBACK_CONTEXT_CAPACITY_EXCEEDED:
RPC ACK callback context was evicted because capacity was exceeded,
connectionId={}, failedFutureCount={}, contextSizeBefore={},
contextSizeAfter={}, maxContextSize={}

Please avoid logging every request ID individually because capacity pressure may affect many futures and create a log storm.

Prefer an existing rate-limited logging mechanism if one is available. At minimum:

• emit WARN only when the removed context contains pending futures;
• keep empty-context trimming at DEBUG level;
• include the capacity reason in each affected future’s TimeoutException, even if the WARN log is rate-limited.

The exception should clearly state that the timeout was caused by local capacity eviction:

RPC ACK future was evicted because callback context capacity was exceeded,
connectionId=..., requestId=..., maxContextSize=1000000

7. Add boundary and concurrency tests

Please expose a package-private trim overload with a small test capacity instead of allocating one million contexts:

static void trimCallbackContextIfNecessary(int maxSize)

Tests should verify:

1. the outer map returns to the configured limit;
2. a future is either registered in the current context or already completed;
3. a future is never detached and left pending;
4. registration racing with trim does not lose a future;
5. ACK racing with trim completes a future at most once;
6. one failing callback does not prevent other futures from being cleaned;
7. the capacity-triggered TimeoutException contains the dedicated reason;
8. duplicate request ID behavior remains unchanged.

---

Option 2 — Incorporate the complete bounded-map implementation as third-party source

If preserving the original concurrent LRU and eviction behavior exactly is considered more important than minimizing the source diff, another acceptable option is to incorporate the complete implementation into Nacos.

A suitable package name would be:

com.alibaba.nacos.core.remote.thirdparty.clhm

thirdparty is a conventional package name that clearly communicates external provenance without using an informal name such as copy.

1. Incorporate the complete package, not only the main class

ConcurrentLinkedHashMap depends on several companion types. Please incorporate the complete clhm source package from the exact version previously used by Nacos:

ConcurrentLinkedHashMap.java
EvictionListener.java
GuardedBy.java
Immutable.java
LinkedDeque.java
NotThreadSafe.java
ThreadSafe.java
Weigher.java
Weighers.java
package-info.java

Source:

https://github.com/sofastack/sofa-hessian/tree/v3.3.6/src/main/java/com/alipay/hessian/clhm

The source should be relocated to:

core/src/main/java/com/alibaba/nacos/core/remote/thirdparty/clhm

RpcAckCallbackSynchronizer should then import:

com.alibaba.nacos.core.remote.thirdparty.clhm.ConcurrentLinkedHashMap

This preserves the existing bounded concurrent LRU and eviction-listener semantics without reintroducing the whole Hessian dependency.

2. Preserve copyright and license headers

Every incorporated file must retain the original header, including:

Copyright 2010 Benjamin Manes

and the complete Apache License 2.0 notice.

Do not replace the original copyright header with the standard Alibaba copyright header.

Because the package name and integration points are modified, each modified file should also carry a prominent provenance/modification notice, for example:

/*
 * Incorporated from sofa-hessian v3.3.6.
 * The package was relocated for use as an internal Nacos implementation.
 * See https://github.com/sofastack/sofa-hessian/tree/v3.3.6/src/main/java/com/alipay/hessian/clhm.
 */

3. Explain the reason in the main class Javadoc

Please add a class-level explanation to ConcurrentLinkedHashMap, for example:

/**
 * A bounded concurrent map incorporated from the {@code clhm} implementation
 * distributed with sofa-hessian 3.3.6.
 *
 * <p>SOFA-JRaft 1.4.1 no longer introduces sofa-hessian transitively, while
 * {@code RpcAckCallbackSynchronizer} still requires the existing bounded
 * concurrent LRU and eviction-listener semantics. The implementation is kept
 * locally under the Nacos third-party namespace to avoid reintroducing the
 * complete Hessian dependency solely for this utility and to preserve the
 * previous runtime behavior.
 *
 * <p>The original source is available at:
 * https://github.com/sofastack/sofa-hessian/tree/v3.3.6/src/main/java/com/alipay/hessian/clhm
 *
 * <p>The original copyright and Apache License 2.0 notices are retained.
 */

It would also be useful to document the provenance in package-info.java.

4. Update repository-level attribution and release files

Because this source will be compiled into and distributed with Nacos, please update the following repository files.

Root NOTICE

Add an attribution entry similar to:

This product includes a relocated and minimally adapted implementation of
ConcurrentLinkedHashMap from sofa-hessian 3.3.6.

Source:
https://github.com/sofastack/sofa-hessian/tree/v3.3.6/src/main/java/com/alipay/hessian/clhm

Copyright 2010 Benjamin Manes

Licensed under the Apache License, Version 2.0.

The root NOTICE already contains precedent for externally incorporated source, such as InetAddressValidator.

distribution/NOTICE-BIN

Add the same attribution because binary Nacos distributions use NOTICE-BIN rather than the root source NOTICE.

distribution/LICENSE-BIN

Add a component entry identifying the incorporated ConcurrentLinkedHashMap implementation, its source URL, copyright owner, and Apache License 2.0 status.

The Apache 2.0 license text is already present, so duplicating the complete license text should not be necessary, but the incorporated component must be identified.

Root LICENSE

No change is expected because both Nacos and the incorporated implementation use Apache License 2.0.

5. Update formatting/checkstyle exclusions when preserving upstream source

The current root pom.xml excludes generated and externally maintained sources such as common/packagescan from Spotless and/or Checkstyle.

If the incorporated source is intended to remain close to upstream, add:

**/core/remote/thirdparty/clhm/**

to the relevant Checkstyle and Spotless exclusions in the root pom.xml.

Also update:

style/codeStyle.md

to document that this package is externally maintained third-party source and intentionally excluded from automatic formatting.

Do not add an Apache RAT exclusion. The original files already contain valid Apache License 2.0 headers, and RAT should continue validating them.

SpotBugs should also continue to run unless a concrete false positive requires a narrowly scoped exclusion.

6. Add and port tests

The sofa-hessian v3.3.6 package contains a small ConcurrentLinkedHashMapTest, but it is not sufficient by itself for the Nacos usage.

Please add tests under:

core/src/test/java/com/alibaba/nacos/core/remote/thirdparty/clhm

and retain/update RpcAckCallbackSynchronizerTest.

The tests should cover:

• maximum weighted capacity;
• LRU/access-order eviction;
• eviction-listener invocation;
• concurrent get, putIfAbsent, remove, and eviction;
• explicit removal not being treated as capacity eviction;
• callback cleanup under capacity pressure;
• capacity-eviction logging and exception messages.

7. Logging is still required with this option

Even when the original ConcurrentLinkedHashMap is retained, the eviction listener in RpcAckCallbackSynchronizer currently reports only a generic empty TimeoutException.

Please add the same capacity-specific WARN and exception message described in Option 1. The log belongs in RpcAckCallbackSynchronizer, not inside the incorporated third-party map implementation.

This keeps the third-party implementation generic while making the Nacos-specific reason observable.

8. Required validation

Please run at least:

mvn -pl core spotless:apply
mvn -pl core spotless:check
mvn -pl core -am \
    -Dtest=RpcAckCallbackSynchronizerTest,ConcurrentLinkedHashMapTest \
    -Dsurefire.failIfNoSpecifiedTests=false test
mvn -pl core -am -DskipTests compile
mvn apache-rat:check
mvn checkstyle:check

Also confirm through the dependency tree that com.alipay.sofa:hessian is no longer present.

---

Recommendation

Option 1 is preferred because it has a smaller maintenance surface, no additional dependency, and keeps the change limited to RpcAckCallbackSynchronizer.

Option 2 is acceptable when exact preservation of the previous concurrent LRU behavior is required. If Option 2 is selected, please incorporate the complete package, retain all original copyright/license information, document why the source is maintained locally, and update both source and binary distribution attribution files.

[jay666mnj]

Request changes: preserve bounded callback behavior without silently losing ACK futures

Replacing the transitive com.alipay.sofa:hessian usage is reasonable, but the current manual trimming implementation is not concurrency-equivalent to the original ConcurrentLinkedHashMap.

There is a race between context initialization, future registration, and trimming:

1. initContextIfNecessary() inserts and returns a context.
2. trimCallbackContextIfNecessary() removes that context from CALLBACK_CONTEXT.
3. syncCallback() still holds the removed map and inserts a future into it.
4. The ACK path later looks up the connection through CALLBACK_CONTEXT and cannot find the detached context.
5. The future is not completed by the trim operation and eventually appears as an ordinary request timeout.

This is misleading during incident investigation because the client may have returned the ACK normally, while the server discarded the matching context because of capacity pressure.

Please consider one of the following two options.

Option 1 — Keep the JDK-only replacement and make trimming concurrency-safe

This is the preferred option because it keeps the change local to RpcAckCallbackSynchronizer, introduces no dependency, and avoids maintaining a complete third-party concurrent cache implementation.

1. Preserve the public field type

CALLBACK_CONTEXT is public and currently declared as Map. Changing the declared field type to ConcurrentMap may introduce binary compatibility concerns.

Please keep the public declaration and back it with a private concurrent store:

private static final ConcurrentMap<String, Map<String, DefaultRequestFuture>>
        CALLBACK_CONTEXT_STORE = new ConcurrentHashMap<>(128);

public static final Map<String, Map<String, DefaultRequestFuture>> CALLBACK_CONTEXT =
        CALLBACK_CONTEXT_STORE;

2. Use ConcurrentHashMap for the inner map

The per-connection map is concurrently accessed by request, ACK and timeout threads. Retaining HashMap is unsafe.

Map<String, DefaultRequestFuture> context =
        new ConcurrentHashMap<>(128);

3. Verify context ownership after future registration

After inserting a future, verify that the context is still the current value associated with the connection ID. If it was removed by trimming, withdraw the future from the detached context and retry.

while (true) {
    Map<String, DefaultRequestFuture> context =
        initContextIfNecessary(connectionId);
    DefaultRequestFuture previous = context.putIfAbsent(requestId, future);

    if (CALLBACK_CONTEXT_STORE.get(connectionId) == context) {
        if (previous == null) {
            return;
        }
        throw new NacosException(
            NacosException.INVALID_PARAM, "request id conflict");
    }

    if (previous == null && !context.remove(requestId, future)) {
        // ACK, timeout, or trim already owns completion of this future.
        return;
    }

    // The future was removed from a detached context; retry registration.
}

Normally this loop executes once. A retry is only needed when registration races with trimming near the capacity limit.

4. Claim each future before completing it

After removing a context from the outer map, trim must use conditional removal to obtain ownership of each future:

if (removed.remove(requestId, future)) {
    future.setFailResult(new TimeoutException(...));
}

This gives deterministic ownership:

• if ACK removes the future first, ACK completes it;
• if trim removes the future first, trim completes it with the capacity error;
• timeout and clearFuture follow the same ownership rule;
• the future cannot be completed twice by ACK and trim.

Please catch failures independently for each future. One callback throwing an exception must not stop cleanup of the remaining futures or weaken the capacity bound.

private static int failRemovedContext(
        String connectionId,
        Map<String, DefaultRequestFuture> removed) {
    if (removed == null) {
        return 0;
    }

    int failedCount = 0;
    for (Map.Entry<String, DefaultRequestFuture> entry : removed.entrySet()) {
        String requestId = entry.getKey();
        DefaultRequestFuture future = entry.getValue();
        if (removed.remove(requestId, future)) {
            failedCount++;
            try {
                future.setFailResult(new TimeoutException(
                    "RPC ACK future was evicted because callback context capacity "
                        + "was exceeded, connectionId=" + connectionId
                        + ", requestId=" + requestId
                        + ", maxContextSize=" + MAX_CALLBACK_CONTEXT_SIZE));
            } catch (Throwable throwable) {
                Loggers.REMOTE_DIGEST.warn(
                    "Failed to notify an evicted RPC ACK future, "
                        + "connectionId={}, requestId={}",
                    connectionId, requestId, throwable);
            }
        }
    }
    return failedCount;
}

5. Keep the hard capacity bound

The outer map can still use simple unordered eviction:

private static void trimCallbackContextIfNecessary() {
    while (CALLBACK_CONTEXT_STORE.size() > MAX_CALLBACK_CONTEXT_SIZE) {
        Iterator<String> iterator = CALLBACK_CONTEXT_STORE.keySet().iterator();
        if (!iterator.hasNext()) {
            return;
        }

        int sizeBefore = CALLBACK_CONTEXT_STORE.size();
        String connectionId = iterator.next();
        Map<String, DefaultRequestFuture> removed =
            CALLBACK_CONTEXT_STORE.remove(connectionId);

        int failedFutureCount = failRemovedContext(connectionId, removed);
        logCapacityTrimIfNecessary(
            connectionId, failedFutureCount, sizeBefore,
            CALLBACK_CONTEXT_STORE.size());
    }
}

This does not preserve exact LRU ordering, but it preserves the important bounded-capacity and eviction-notification behavior without silently detaching futures.

Strict concurrent LRU cannot reasonably be reproduced with a small JDK-only implementation while also keeping the implementation simple and avoiding additional dependencies.

6. Add explicit capacity-trim diagnostics

Please do not complete evicted futures with an empty TimeoutException. A capacity-triggered eviction is not an ordinary remote ACK timeout and must be distinguishable during troubleshooting.

When a trimmed context contains pending futures, emit a WARN log containing:

• a stable reason such as RPC_ACK_CALLBACK_CONTEXT_CAPACITY_EXCEEDED;
• the evicted connection ID;
• the number of pending futures claimed by trim;
• context size before and after trimming;
• the configured maximum context size.

For example:

RPC_ACK_CALLBACK_CONTEXT_CAPACITY_EXCEEDED:
RPC ACK callback context was evicted because capacity was exceeded,
connectionId={}, failedFutureCount={}, contextSizeBefore={},
contextSizeAfter={}, maxContextSize={}

Please avoid logging every request ID individually because capacity pressure may affect many futures and create a log storm.

Prefer an existing rate-limited logging mechanism if one is available. At minimum:

• emit WARN only when the removed context contains pending futures;
• keep empty-context trimming at DEBUG level;
• include the capacity reason in each affected future’s TimeoutException, even if the WARN log is rate-limited.

The exception should clearly state that the timeout was caused by local capacity eviction:

RPC ACK future was evicted because callback context capacity was exceeded,
connectionId=..., requestId=..., maxContextSize=1000000

7. Add boundary and concurrency tests

Please expose a package-private trim overload with a small test capacity instead of allocating one million contexts:

static void trimCallbackContextIfNecessary(int maxSize)

Tests should verify:

1. the outer map returns to the configured limit;
2. a future is either registered in the current context or already completed;
3. a future is never detached and left pending;
4. registration racing with trim does not lose a future;
5. ACK racing with trim completes a future at most once;
6. one failing callback does not prevent other futures from being cleaned;
7. the capacity-triggered TimeoutException contains the dedicated reason;
8. duplicate request ID behavior remains unchanged.

Option 2 — Incorporate the complete bounded-map implementation as third-party source

If preserving the original concurrent LRU and eviction behavior exactly is considered more important than minimizing the source diff, another acceptable option is to incorporate the complete implementation into Nacos.

A suitable package name would be:

com.alibaba.nacos.core.remote.thirdparty.clhm

thirdparty is a conventional package name that clearly communicates external provenance without using an informal name such as copy.

1. Incorporate the complete package, not only the main class

ConcurrentLinkedHashMap depends on several companion types. Please incorporate the complete clhm source package from the exact version previously used by Nacos:

ConcurrentLinkedHashMap.java
EvictionListener.java
GuardedBy.java
Immutable.java
LinkedDeque.java
NotThreadSafe.java
ThreadSafe.java
Weigher.java
Weighers.java
package-info.java

Source:

https://github.com/sofastack/sofa-hessian/tree/v3.3.6/src/main/java/com/alipay/hessian/clhm

The source should be relocated to:

core/src/main/java/com/alibaba/nacos/core/remote/thirdparty/clhm

RpcAckCallbackSynchronizer should then import:

com.alibaba.nacos.core.remote.thirdparty.clhm.ConcurrentLinkedHashMap

This preserves the existing bounded concurrent LRU and eviction-listener semantics without reintroducing the whole Hessian dependency.

2. Preserve copyright and license headers

Every incorporated file must retain the original header, including:

Copyright 2010 Benjamin Manes

and the complete Apache License 2.0 notice.

Do not replace the original copyright header with the standard Alibaba copyright header.

Because the package name and integration points are modified, each modified file should also carry a prominent provenance/modification notice, for example:

/*
 * Incorporated from sofa-hessian v3.3.6.
 * The package was relocated for use as an internal Nacos implementation.
 * See https://github.com/sofastack/sofa-hessian/tree/v3.3.6/src/main/java/com/alipay/hessian/clhm.
 */

3. Explain the reason in the main class Javadoc

Please add a class-level explanation to ConcurrentLinkedHashMap, for example:

/**
 * A bounded concurrent map incorporated from the {@code clhm} implementation
 * distributed with sofa-hessian 3.3.6.
 *
 * <p>SOFA-JRaft 1.4.1 no longer introduces sofa-hessian transitively, while
 * {@code RpcAckCallbackSynchronizer} still requires the existing bounded
 * concurrent LRU and eviction-listener semantics. The implementation is kept
 * locally under the Nacos third-party namespace to avoid reintroducing the
 * complete Hessian dependency solely for this utility and to preserve the
 * previous runtime behavior.
 *
 * <p>The original source is available at:
 * https://github.com/sofastack/sofa-hessian/tree/v3.3.6/src/main/java/com/alipay/hessian/clhm
 *
 * <p>The original copyright and Apache License 2.0 notices are retained.
 */

It would also be useful to document the provenance in package-info.java.

4. Update repository-level attribution and release files

Because this source will be compiled into and distributed with Nacos, please update the following repository files.

Root NOTICE

Add an attribution entry similar to:

This product includes a relocated and minimally adapted implementation of
ConcurrentLinkedHashMap from sofa-hessian 3.3.6.

Source:
https://github.com/sofastack/sofa-hessian/tree/v3.3.6/src/main/java/com/alipay/hessian/clhm

Copyright 2010 Benjamin Manes

Licensed under the Apache License, Version 2.0.

The root NOTICE already contains precedent for externally incorporated source, such as InetAddressValidator.

distribution/NOTICE-BIN

Add the same attribution because binary Nacos distributions use NOTICE-BIN rather than the root source NOTICE.

distribution/LICENSE-BIN

Add a component entry identifying the incorporated ConcurrentLinkedHashMap implementation, its source URL, copyright owner, and Apache License 2.0 status.

The Apache 2.0 license text is already present, so duplicating the complete license text should not be necessary, but the incorporated component must be identified.

Root LICENSE

No change is expected because both Nacos and the incorporated implementation use Apache License 2.0.

5. Update formatting/checkstyle exclusions when preserving upstream source

The current root pom.xml excludes generated and externally maintained sources such as common/packagescan from Spotless and/or Checkstyle.

If the incorporated source is intended to remain close to upstream, add:

**/core/remote/thirdparty/clhm/**

to the relevant Checkstyle and Spotless exclusions in the root pom.xml.

Also update:

style/codeStyle.md

to document that this package is externally maintained third-party source and intentionally excluded from automatic formatting.

Do not add an Apache RAT exclusion. The original files already contain valid Apache License 2.0 headers, and RAT should continue validating them.

SpotBugs should also continue to run unless a concrete false positive requires a narrowly scoped exclusion.

6. Add and port tests

The sofa-hessian v3.3.6 package contains a small ConcurrentLinkedHashMapTest, but it is not sufficient by itself for the Nacos usage.

Please add tests under:

core/src/test/java/com/alibaba/nacos/core/remote/thirdparty/clhm

and retain/update RpcAckCallbackSynchronizerTest.

The tests should cover:

• maximum weighted capacity;
• LRU/access-order eviction;
• eviction-listener invocation;
• concurrent get, putIfAbsent, remove, and eviction;
• explicit removal not being treated as capacity eviction;
• callback cleanup under capacity pressure;
• capacity-eviction logging and exception messages.

7. Logging is still required with this option

Even when the original ConcurrentLinkedHashMap is retained, the eviction listener in RpcAckCallbackSynchronizer currently reports only a generic empty TimeoutException.

Please add the same capacity-specific WARN and exception message described in Option 1. The log belongs in RpcAckCallbackSynchronizer, not inside the incorporated third-party map implementation.

This keeps the third-party implementation generic while making the Nacos-specific reason observable.

8. Required validation

Please run at least:

mvn -pl core spotless:apply
mvn -pl core spotless:check
mvn -pl core -am \
    -Dtest=RpcAckCallbackSynchronizerTest,ConcurrentLinkedHashMapTest \
    -Dsurefire.failIfNoSpecifiedTests=false test
mvn -pl core -am -DskipTests compile
mvn apache-rat:check
mvn checkstyle:check

Also confirm through the dependency tree that com.alipay.sofa:hessian is no longer present.

Recommendation

Option 1 is preferred because it has a smaller maintenance surface, no additional dependency, and keeps the change limited to RpcAckCallbackSynchronizer.

Option 2 is acceptable when exact preservation of the previous concurrent LRU behavior is required. If Option 2 is selected, please incorporate the complete package, retain all original copyright/license information, document why the source is maintained locally, and update both source and binary distribution attribution files.

Thanks for the detailed review.

I have updated this PR following Option 1:

• Kept the public CALLBACK_CONTEXT as Map, backed by a private ConcurrentMap.
• Changed the per-connection callback context to ConcurrentHashMap.
• Added ownership verification after registering the future to avoid leaving pending futures in a detached context.
• Updated capacity trimming to claim each future with conditional removal before completing it.
• Kept the hard capacity bound with unordered eviction.
• Added the stable capacity reason RPC_ACK_CALLBACK_CONTEXT_CAPACITY_EXCEEDED.
• Added WARN logs only when pending futures are failed, and DEBUG logs for empty context trimming.
• Included the capacity reason in the TimeoutException.
• Added boundary and concurrency tests for registration, trimming, ACK race, duplicate request ID, callback failure, and capacity timeout behavior.

Verified locally:

• mvn -pl core spotless:check
• mvn -pl core -am -Dtest=RpcAckCallbackSynchronizerTest -Dsurefire.failIfNoSpecifiedTests=false test
• mvn -pl core -am -DskipTests compile
• mvn apache-rat:check
• mvn checkstyle:check
• Confirmed com.alipay.sofa:hessian / com.caucho:hessian are absent from the dependency tree.

[KomachiSion]

Thanks for the update. I re-reviewed commit 9eb2216. The private ConcurrentMap plus the
public Map alias preserves the existing field descriptor, and the publish-check /
conditional-remove retry in syncCallback closes the detached-context registration race.
I also did not find a newly introduced unbounded memory-retention path.

There are still two changes I suggest making before approval:

1. Only the thread that actually inserts a new connection context should invoke trim.

Currently, every thread that initially observes a missing connectionId calls
trimCallbackContextIfNecessary(), including threads whose putIfAbsent loses the race.
If several requests concurrently initialize the same connection while the store is just
over capacity, one new context can therefore cause multiple unrelated contexts to be
evicted and their pending futures to fail.

Please return the existing context immediately for the losing path, for example:

Map<String, DefaultRequestFuture> existingContext =
        CALLBACK_CONTEXT_STORE.putIfAbsent(connectionId, newContext);
if (existingContext != null) {
    return existingContext;
}
trimCallbackContextIfNecessary();
return newContext;

This is also cheaper because it avoids redundant size checks and trim iterations.

2. Please rate-limit the capacity WARN log.

Once the store reaches its maximum size, every newly inserted connection context may
produce a WARN. Under sustained connection churn this can become a log storm and add
significant I/O pressure exactly when the server is already overloaded. A small
AtomicLong/CAS time gate, for example one warning per minute, should be sufficient and
does not require another dependency. Please keep
RPC_ACK_CALLBACK_CONTEXT_CAPACITY_EXCEEDED in both the sampled log and the future
exception so the capacity loss remains distinguishable from an ordinary RPC timeout.

One residual issue should also be documented or covered separately:
DefaultRequestFuture.setFailResult() does not cancel its scheduled timeout task. An
async future failed by capacity trimming can therefore receive the capacity exception
immediately and another ordinary timeout callback at its original deadline. This
behavior already existed in the old removal listener, so I do not consider it a new
regression in this PR, but the current at-most-once test does not cover it because it
asserts immediately and completionCount <= 1 also permits zero completions. Please at
least use assertEquals(1, completionCount.get()), narrow the test name to the ACK/trim
ownership race, and cancel callback futures during test cleanup.

[KomachiSion]

@jay666mnj
Thanks for the updates. I performed a full pass over the latest head
(6b8affd), including context initialization,
future registration, ACK handling, timeout cleanup, disconnect cleanup, capacity
trimming, logging, tests, and the JRaft 1.4.1 dependency change.

The previous redundant-trim issue is fixed, the public Map field descriptor is
preserved, the inner maps are now concurrent, and the ACK/trim conditional removal
correctly establishes ownership of a pending future.

To avoid another sequence of incremental fixes, I suggest completing the PR with the
following final set of changes.

1. Protect the context inserted by the current initialization from its own trim.

initContextIfNecessary() currently inserts an empty context and trims before
syncCallback() registers the future. Because ConcurrentHashMap iteration is unordered,
trim may select the newly inserted context itself.

The request can then repeatedly:

• insert the context;
• remove that same context during trim;
• register into the detached context;
• fail the ownership check;
• retry with the same connectionId.

Since the key hash and the remaining map state are unchanged, this can repeat without
making progress.

Please pass the exact newly inserted context instance to the production trim method and
skip that instance while selecting the victim:

if (existingContext != null) {
    return existingContext;
}
trimCallbackContextIfNecessary(MAX_CALLBACK_CONTEXT_SIZE, newContext);
return newContext;

The existing package-private trim(maxSize) overload can delegate with a null protected
context for boundary tests.

2. Remove the selected outer mapping conditionally.

Please iterate over map entries, retain both the candidate key and expected context, and
claim the mapping with:

CALLBACK_CONTEXT_STORE.remove(victimConnectionId, victimContext)

If this returns false, retry victim selection. An unconditional remove(key) can delete a
replacement context installed after the iterator observed an older mapping for the same
key.

Together, protected-context selection and conditional outer removal provide a clear
ownership rule without adding a lock, dependency, or connection-manager change.

3. Separate the non-empty and rate-limit logging branches.

The current condition:

if (failedFutureCount > 0 && shouldLogCapacityWarn()) {
    ...
} else {
    // logs an empty context
}

incorrectly reports a non-empty eviction as empty whenever the WARN is suppressed by the
time gate. With DEBUG enabled it can also produce one log event per trim.

Please return immediately for empty contexts and independently apply the WARN gate:

if (failedFutureCount <= 0) {
    return;
}
if (!shouldLogCapacityWarn()) {
    return;
}
// WARN

There is no need to log every empty-context eviction.

4. Prefer one fixed-size aggregate counter for suppressed failures.

An AtomicLong failedFutureCountSinceLastWarn is sufficient. Increment it before applying
the time gate and reset it with getAndSet(0) when a warning is emitted. The warning can
then include the total number of affected futures since the previous warning plus one
sample connectionId.

This remains O(1) memory, retains no connectionId collection, creates no scheduled task,
and provides the impact size without a log storm. System.nanoTime() is preferable for
the interval calculation so a wall-clock adjustment cannot suppress warnings for an
unexpectedly long period.

5. Make the concurrency tests deterministic enough to detect regressions.

Please add tests that verify:

• trim never selects the protected newly inserted context;
• outer removal does not delete a replacement mapping;
• the final outer-map size is within the configured bound;
• a registered future is either present in the current context or already completed;
• ACK and trim claim a future exactly once;
• one throwing callback does not stop cleanup of other futures;
• the capacity exception contains the dedicated reason;
• duplicate request IDs retain their existing behavior.

The current registration/trim test submits the trim task last to the same fixed-size
executor, so most registration tasks finish before trim starts. Please use a separate
trim executor or submit trim first, and retain/get every submitted Future so task
exceptions are not silently ignored.

I do not suggest expanding this PR to redesign DefaultRequestFuture completion,
ConnectionManager, or disconnect semantics. The scheduled-timeout double-callback and
sync-send-failure cleanup concerns predate this dependency replacement and should be
tracked separately. The accepted semantic difference for this option is that eviction is
unordered rather than approximate LRU.

Finally, JRaft 1.4.1 contains Raft behavior changes in addition to dependency cleanup,
including election persistence, read-index, apply-pipeline, and replicator changes.
Please run the complete core unit-test suite, confirm the effective dependency tree, and
provide at least a basic CP/JRaft cluster smoke result for leader election, replication,
snapshot, and restart. If rolling-upgrade validation is not available, please state that
explicitly.

With the above code changes, tests, and dependency/CP validation completed, I do not see
a need for additional dependencies, copied third-party source, ConnectionManager
changes, or repository-level NOTICE/spec changes.

[nacos-community]

Summary

Upgrades SOFA-JRaft to 1.4.1 and replaces the removed ConcurrentLinkedHashMap-based callback store with a ConcurrentHashMap plus manual trim; the dependency bump itself is fine, but the replacement eviction path introduces concurrency and ACK-lifecycle regressions in a core module, so changes are requested before merge.

Findings

• [Warning] core/src/main/java/com/alibaba/nacos/core/remote/RpcAckCallbackSynchronizer.java:148 — at the capacity limit, trimCallbackContextIfNecessary can evict the newContext that was just published; syncCallback then removes its own registration and retries without a progress guarantee, so a push can spin indefinitely instead of registering. Exclude the new context from eviction or evict an existing context before publishing it.
• [Warning] core/src/main/java/com/alibaba/nacos/core/remote/RpcAckCallbackSynchronizer.java:175 — iterating ConcurrentHashMap keys selects an arbitrary context, replacing the previous LRU eviction behavior; an active connection with pending ACKs can be failed while idle contexts remain. Preserve a recency/idle eviction policy or explicitly document and test this semantic change.
• [Warning] core/src/main/java/com/alibaba/nacos/core/remote/RpcAckCallbackSynchronizer.java:182 — the unconditional remove(connectionId) can delete a context recreated after clearContext between iterator.next() and this call, then fail that replacement's live ACK futures. Iterate entries and use remove(connectionId, expectedContext) before failing futures.

Suggestions

• Make eviction select an entry and conditionally remove that exact map instance before completing any futures.
• Add deterministic tests with an injectable capacity to cover self-eviction during registration and clear/recreate racing with trim.
• Retain or deliberately replace the prior LRU behavior with a documented policy that avoids evicting active connections first.

---

Automated review by github-manager-bot

[nacos-community]

At the capacity limit, trim can evict the newContext just published here; syncCallback then removes its own registration and retries without a progress guarantee, so a push can spin indefinitely instead of registering. Exclude the new context from eviction or evict an existing context before publishing it.

[nacos-community]

Iterating ConcurrentHashMap keys selects an arbitrary context, replacing the previous ConcurrentLinkedHashMap LRU eviction behavior; an active connection with pending ACKs can be failed while idle contexts remain. Preserve a recency/idle eviction policy or explicitly document and test this semantic change.

[nacos-community]

This unconditional remove can delete a context recreated after clearContext between iterator.next() and this call, then fail that replacement's live ACK futures. Iterate entries and use remove(connectionId, expectedContext) before failing futures.