fix: incorrect cross-cluster call warning when clusterName is not set#4287
Conversation
|
Thanks for the fix. One follow-up concern: this warning is still emitted from the request path in Would you consider adding some form of throttling here, for example:
The current fix removes the false positives, which is great, but rate-limiting the warning would make the behavior much safer in sustained fallback scenarios. |
a5e0e0e to
6b69710
Compare
|
@uuuyuqi Good point! I've updated this PR to include log throttling — the cross-cluster warning now logs at most once per 5 minutes per Added test: |
ab76748 to
e0d2315
Compare
Move the "A cross-cluster call occurs" warning from the outer else block (which triggers when clusterName is blank) into the inner else block (which triggers when clusterName IS set but no same-cluster instances are found). Additionally, throttle the cross-cluster warning to log at most once per 5 minutes per (serviceId, clusterName) to prevent log flooding under sustained cross-cluster fallback traffic. Fixes alibaba#4020
e0d2315 to
1ddb22a
Compare
Describe what this PR does / why we need it
In
NacosLoadBalancer.getInstanceResponse(), the "A cross-cluster call occurs" warning log is incorrectly placed in the outerelseblock, which triggers whenclusterNameis blank (null or empty). Since PR #3771 removed the default value forclusterName, this warning fires on every load balancing request when users don't explicitly configure a cluster name.The warning should only appear when
clusterNameIS configured but no same-cluster instances are found — that is the actual cross-cluster call scenario.Does this pull request fix one issue?
Fixes #4020
Describe how you did it
Moved the
elseblock with the warning log from the outerif (StringUtils.isNotBlank(clusterName))into the innerif (!CollectionUtils.isEmpty(sameClusterInstances)).Before (incorrect):
After (correct):
Describe how to verify it
spring.cloud.nacos.discovery.cluster-nameTests Added
blankClusterNameShouldNotWarnCrossCluster()emptyClusterNameShouldNotWarnCrossCluster()sameClusterShouldSelectSameClusterInstance()noSameClusterInstancesShouldStillReturnResponse()emptyServiceInstancesShouldReturnEmptyResponse()Special notes for reviews
PR #4021 proposed the same fix for the
2021.xbranch. This PR applies it to the current2025.1.xbranch with comprehensive unit tests.