Skip to content

Nacos instance register: a weight-only change produces phantom ADDED+DELETED events that permanently delete the upstream row #7169

Description

@Chrisp12138

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,
    • DELETEDdiscoveryUpstreamMapper.deleteByUrl(...)row deleted,
    • UPDATEDupdateDiscoveryHandlerIdAndUrl(...)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
  1. 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:

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():

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).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions