sunnysabor commented on PR #7882: URL: https://github.com/apache/shenyu/pull/7882#issuecomment-6290987044
Reviewed this against the production failure sequence described in #7881. From the incident perspective, this PR fixes the reported divide outage path: - the complete registry snapshot now refreshes `UPSTREAM_MAP`, so a newly discovered instance is no longer absent from the Admin health-check cache; - an in-flight health check cannot overwrite a newer registry replacement because the checked list identity is retained and compared; - reconciliation excludes instances that actually failed the current check while preserving healthy database instances that were not part of that stale check. This prevents the original sequence where the old instance failed, the new instance existed in the database/gateway snapshot but not in `UPSTREAM_MAP`, and the health checker consequently published an authoritative empty snapshot. I found no blocking correctness issue for #7881. Two non-blocking follow-ups are still worth considering: 1. `UPSTREAM_MAP.compute(...)` currently holds the per-key map lock across database I/O and event dispatch. Moving the DB writes and publishing outside the remapping function would reduce contention and future re-entrancy/deadlock risk. 2. `DiscoveryDataChangedEventSyncListener` is instantiated directly by `AbstractDiscoveryProcessor`, so its own `@Transactional` annotation does not by itself create a proxied transaction. The non-transactional path is correctly ordered, and an ambient-transaction path is covered, but a Spring integration test through the real listener construction/callback path would clarify the actual production transaction semantics. Also, the cache refresh listener is currently limited to `PluginEnum.DIVIDE`, which is sufficient for this incident but should be stated as the fix scope. -- 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]
