sunnysabor opened a new issue, #7881:
URL: https://github.com/apache/shenyu/issues/7881
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current Behavior
### What happened?
During a rolling deployment, a new service instance was discovered
successfully. Shortly afterward, the old instance failed the Admin upstream
health check.
Although the new instance had already been persisted and synchronized
through DISCOVER_UPSTREAM, UpstreamCheckService still held only the old
instance in its internal UPSTREAM_MAP.
When the old instance failed its health check:
1. UpstreamCheckService produced an empty successList.
2. updateSelectorHandler(selectorId, emptyList) queried the current
discovery upstreams from the database.
3. It then filtered the database result using the stale, empty aliveList.
4. This removed the newly discovered healthy instance from the outgoing list.
5. Admin published an empty DISCOVER_UPSTREAM / UPDATE snapshot.
6. Gateway instances treated the empty list as an authoritative full
snapshot and removed all upstream caches for the selector.
7. DividePlugin could no longer find a healthy upstream and returned:
~~~json
{
"code": -119,
"message": "Can not find healthy upstream url, please check your
configuration!"
}
~~~
A manual discovery/divide synchronization republished the current healthy
instance and immediately restored traffic.
### Root cause
DiscoveryDataChangedEventSyncListener updates the database and publishes a
DataChangedEvent after registry ADDED, UPDATED, and DELETED events, but this
path does not publish a DiscoveryStreamUpdatedEvent.
UpstreamCheckService refreshes UPSTREAM_MAP through its
DiscoveryStreamUpdatedEvent listener. LocalDiscoveryProcessor.changeUpstream()
publishes that internal event, while the registry event path does not.
Therefore the database and gateway snapshot can contain a newly discovered
instance while the Admin health-check map remains stale.
The stale state becomes destructive in updateSelectorHandler(): it queries
the full database snapshot and then removes every item not present in the local
aliveList. If the stale map only contains the old instance and that instance
fails, aliveList is empty, so newly discovered instances that were never
checked in this cycle are also removed from the outgoing snapshot.
### Impact
This can cause a complete outage for one selector during rolling
deployments, scaling events, pod-IP replacement, or instance migration. All
gateways receiving the empty snapshot may return -119 even though the registry
and Admin database already contain a healthy replacement instance.
### Expected Behavior
When a new instance has already been discovered and persisted, failure of an
old instance must not remove the new instance from the published discovery
snapshot.
For this sequence:
~~~text
Admin starts with upstream A
-> registry discovers upstream B
-> upstream A becomes unavailable
~~~
the resulting snapshot must still contain B. It must not become empty unless
the authoritative discovery state genuinely has no upstreams.
### Steps To Reproduce
1. Enable Admin upstream health checking.
2. Configure a divide selector backed by a dynamic registry such as Eureka.
3. Start Admin while only upstream A exists, so
UpstreamCheckService.UPSTREAM_MAP contains A.
4. Register upstream B after Admin has started.
5. Verify that B is persisted and synchronized to the gateway.
6. Without manually refreshing the Admin health-check cache, stop A or make
its health check fail.
7. Wait for the periodic Admin health check.
8. Observe that Admin publishes an empty DISCOVER_UPSTREAM / UPDATE list.
9. Observe that the gateway clears the selector upstream cache and
DividePlugin returns -119.
10. Trigger a manual discovery synchronization and observe that B is
restored.
### Environment
```markdown
ShenYu version(s): current master
Verified commit: af9d695e8776f0ece545b0118fef8a39fc05191e
Discovery type: Eureka
Gateway sync: WebSocket
```
### Debug logs
Observed event order (identifiers and addresses sanitized):
~~~text
T1: registry events for the selector were received
T2: a new instance B was persisted and synchronized
T3: Admin logged "change unlive" for old instance A
T4: multiple gateway instances started returning -119
T5: both healthyUpstream[selectorId] and unhealthyUpstream[selectorId] were
null
T6: manual discovery/divide synchronization republished B
T7: traffic recovered
~~~
A normal gateway health-check failure generally moves an upstream from the
healthy map to the unhealthy map. Both entries being absent is consistent with
the gateway receiving an empty authoritative snapshot and executing
removeByKey(selectorId).
### Anything else?
### Relevant code
-
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/UpstreamCheckService.java
-
shenyu-admin/src/main/java/org/apache/shenyu/admin/discovery/DiscoveryDataChangedEventSyncListener.java
-
shenyu-admin/src/main/java/org/apache/shenyu/admin/discovery/LocalDiscoveryProcessor.java
-
shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCacheManager.java
-
shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-divide/src/main/java/org/apache/shenyu/plugin/divide/DividePlugin.java
Source links at the verified commit:
-
https://github.com/apache/shenyu/blob/af9d695e8776f0ece545b0118fef8a39fc05191e/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/UpstreamCheckService.java
-
https://github.com/apache/shenyu/blob/af9d695e8776f0ece545b0118fef8a39fc05191e/shenyu-admin/src/main/java/org/apache/shenyu/admin/discovery/DiscoveryDataChangedEventSyncListener.java
-
https://github.com/apache/shenyu/blob/af9d695e8776f0ece545b0118fef8a39fc05191e/shenyu-admin/src/main/java/org/apache/shenyu/admin/discovery/LocalDiscoveryProcessor.java
### Suggested fix
1. After a registry event is successfully committed, replace the matching
UpstreamCheckService.UPSTREAM_MAP entry with the same complete selector
snapshot. The in-memory update should occur after transaction commit.
2. updateSelectorHandler() should only update instances actually checked in
the current cycle. It should not use a possibly stale aliveList to remove
database nodes that were never checked.
3. Keep empty snapshots valid when the authoritative selector genuinely has
no upstreams; skipping every empty update would leave stale gateway entries.
4. Add concurrency coverage for registry events racing with health checks
and for multiple Admin nodes.
### Suggested regression test
1. Initialize the health-check map with old instance A.
2. Simulate a registry event adding B and persist A+B.
3. Make A fail its health check.
4. Assert that the published DISCOVER_UPSTREAM snapshot still contains B and
is not empty.
5. Apply it to UpstreamCacheManager and assert that B remains healthy and
DividePlugin does not return CANNOT_FIND_HEALTHY_UPSTREAM_URL.
### Related issues
- #6144 has a similar rolling-deployment and -119 symptom, but concerns
incremental discovery messages being treated as complete gateway snapshots.
- #6021 concerns unhealthy instances not being removed.
- #6479 concerns deleted upstreams remaining in gateway caches.
- #6502 concerns the health-check scheduler becoming stuck after an async
failure.
- #6904 concerns namespace filtering of Eureka incremental events.
- #7169 concerns phantom Nacos delete events.
This issue is specifically about the registry-event path not refreshing
UpstreamCheckService.UPSTREAM_MAP, followed by a stale aliveList filtering a
newer authoritative database snapshot.
--
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]
