Chrisp12138 opened a new issue, #7169:
URL: https://github.com/apache/shenyu/issues/7169
## Description
With `shenyu-registry-nacos` (Nacos instance register mode), changing **only
the weight** of a live instance in Nacos (console or API) makes the gateway
lose the upstream:
1. `NacosInstanceRegisterRepository#compareInstances` diffs the
previous/current instance lists with `Set.contains(...)`, which relies on Nacos
`Instance#equals()` — and that `equals()` **includes `weight`**.
2. A weight-only change therefore yields, for the *same* `instanceId`:
* `ADDED` (the new-weight instance — "new" only because `equals()`
differs),
* `DELETED` (the old-weight instance),
* `UPDATED` (same `instanceId`, `!equals`).
3. On the admin side, `DiscoveryDataChangedEventSyncListener#syncData0`
processes them in that order, matching rows by `url` (ip:port — unchanged):
* `ADDED` → row already exists → **skipped**,
* `DELETED` → `discoveryUpstreamMapper.deleteByUrl(...)` → **row
deleted**,
* `UPDATED` → `updateDiscoveryHandlerIdAndUrl(...)` → **effect = 0** (row
is already gone).
Net result: the `discovery_upstream` row for a perfectly healthy instance is
permanently deleted, the trailing `DISCOVER_UPSTREAM UPDATE` pushes the removal
to the gateway, and traffic through that upstream 503s/blackholes until the row
is manually re-created or the service re-registers.
## Environment
* ShenYu version(s): 2.7.2-SNAPSHOT (master as of 2026-09); the same
`compareInstances` logic exists in earlier 2.7.x
* Nacos server 3.x with nacos-client 3.x (also reproducible with 2.x —
`Instance#equals()` includes `weight` in both)
* Instance register mode: `shenyu.register.registerType=http` +
websocket/http selectors bound to a Nacos discovery (`shenyu-registry-nacos`
watch path)
## Steps to reproduce
1. Register a service instance via URI report so that a `discovery_upstream`
row exists (e.g. `10.10.10.67:8101`, weight `50`).
2. Change **only the weight** of that instance in the Nacos console (or via
the Nacos API), e.g. `50 → 100`.
3. Observe the admin log:
```
[DiscoveryDataChangedEventSyncListener] Upstream 10.10.10.67:8101 exist
<-- ADDED (new weight, same url) -> skipped
[DiscoveryDataChangedEventSyncListener] DELETE Upstream 10.10.10.67:8101
<-- DELETED (old weight) -> row deleted
[DiscoveryDataChangedEventSyncListener] UPDATE Upstream 10.10.10.67:8101,
effect = 0 <-- UPDATED -> nothing to update
```
4. `SELECT * FROM discovery_upstream WHERE url='10.10.10.67:8101'` → the row
is gone; the gateway no longer routes to the (healthy!) instance.
## Root cause
`NacosInstanceRegisterRepository#compareInstances`:
```java
Set<Instance> addedInstances = currentInstances.stream()
.filter(item -> !previousInstances.contains(item)) //
Instance.equals() includes weight
.collect(Collectors.toSet());
...
Set<Instance> deletedInstances = previousInstances.stream()
.filter(item -> !currentInstances.contains(item)) // same
full-equality diff
.collect(Collectors.toSet());
...
Set<Instance> updatedInstances = currentInstances.stream()
.filter(currentInstance -> ...
currentInstance.getInstanceId().equals(previousInstance.getInstanceId())
&& !currentInstance.equals(previousInstance))
.collect(Collectors.toSet());
```
Any attribute-only change (weight, metadata, enabled, ...) of an existing
instance is mis-classified as a remove+add pair **in addition** to the correct
UPDATED event. Because the admin listener matches upstream rows by `url`
(ip:port), the phantom `DELETED` removes the real row and the trailing
`UPDATED` is a no-op.
## Expected result
An attribute-only change of an existing instance should produce exactly one
`UPDATED` event and never a `DELETED` for a live instance.
## Suggested fix
Key the diff by `instanceId` (fall back to ip:port+cluster when absent)
instead of full `equals()`:
```java
Set<String> previousIds =
previousInstances.stream().map(Instance::getInstanceId).collect(Collectors.toSet());
Set<String> currentIds =
currentInstances.stream().map(Instance::getInstanceId).collect(Collectors.toSet());
Set<Instance> added = currentInstances.stream().filter(i ->
!previousIds.contains(i.getInstanceId()))...
Set<Instance> deleted = previousInstances.stream().filter(i ->
!currentIds.contains(i.getInstanceId()))...
Set<Instance> updated = currentInstances.stream().filter(i ->
previousIds.contains(i.getInstanceId())
&& !i.equals(matchById(previousInstances, i)))...
```
Additionally (defense in depth),
`DiscoveryDataChangedEventSyncListener#syncData0` could treat `UPDATED` as an
**upsert** (insert when the row is missing) so a mis-ordered event pair can no
longer lose a live upstream.
## Note
`EurekaInstanceRegisterRepository#compareInstances` has the same overall
shape and may deserve a check for the equivalent attribute-change case (its
diff type is `InstanceInfo`).
Possibly related (same url-keyed upstream diff family): #6144, #3369.
## Workaround we used
Re-inserted the deleted `discovery_upstream` row manually and pinned weight
changes away from the Nacos path (we now run instance discovery in local
URI-report mode, partly due to this issue).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
