AsperforMias opened a new pull request, #3634: URL: https://github.com/apache/dubbo-go/pull/3634
### Description Fixes #3624 **Root cause.** In application-level service discovery, `serviceDiscoveryRegistry.subscribeAndNotify` runs `serviceDiscovery.AddListener(listener)` fire-and-forget in a goroutine. A transient registry error (reconnect storm, leader election, brief unavailability) was logged once and never retried. Since the initial `GetInstances` snapshot has already been processed, the consumer looks healthy but silently misses every subsequent instance-change push until restart — a single pod can go permanently stale while others stay fine. **Fix.** Mirror the retry discipline of the interface-level nacos subscribe backoff (#3178), at the subscription-channel layer: - One pending retry per `serviceNamesKey` (i.e. per listener): repeated failing subscribes share a single timer instead of stacking. - Exponential backoff 1s → 30s cap + up to 25% jitter; attempts are intentionally unlimited while subscribers remain (a capped count would re-introduce the permanent staleness this fixes). - A successful retry **re-syncs the latest instance snapshot** via `GetInstances`/`OnEvent`, so instance changes missed while the subscription was down are picked up instead of relying on a future push. - Pending retries are canceled when the last subscriber unsubscribes and on `Destroy`; no new timer is armed once the registry is destroyed or no subscriber remains. - Observability: initial failure keeps the existing error log; each failed retry logs one warning with the attempt count and error; a successful recovery logs once; every attempt publishes the existing `NewServerSubscribeEvent` metric. A dedicated "subscription not established" gauge is left to #3356. Two latent bugs in the touched paths are fixed along the way: - `UnSubscribe` removed the notify listener by `url.ServiceKey()` while `SubscribeURL` had registered it under `ServiceKey()+":"+protocol`, so the entry was never removed and subscriber-count-based cleanup could never fire. Both sides now use the same `protocolServiceKeyOf` key. - The `event.Succ = err != nil` flag in the subscribe path was inverted (and a `%s`/`%d` log mismatch next to it). **Verification.** - Unit/regression tests (`registry/servicediscovery/subscribe_retry_test.go`, run with `-race`): first `AddListener` fails → retry establishes the subscription → the consumer receives instance events without a restart; backoff stays in the `[delay, delay+25%]` band and is capped; retries stop after `UnSubscribe` and after `Destroy`, and no timer is armed post-`Destroy`; repeated failing subscribes share one timer. - Real-Nacos e2e (nacos 2.5.1, docker compose, application-level discovery): the consumer wraps the real nacos `ServiceDiscovery` so the first `AddListener` fails (the deterministic variant suggested in the issue). On `upstream/main`: baseline calls work from the initial snapshot, the provider is then replaced, and the consumer stays at `No provider available` for 60s+ until restarted — bug reproduced. On this branch: `established after 1 retries, re-syncing latest instances`, the consumer follows the provider replacement and calls keep succeeding **without a restart**. Note: this adds a small `metadataInfoFetcher` indirection for test fault injection; #3625 introduces the same variable, so whichever PR lands second keeps one copy (trivial conflict). ### Checklist - [x] I confirm the target branch is `develop` - [x] Code has passed local testing - [x] I have added tests that prove my fix is effective or that my feature works -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
