AsperforMias opened a new pull request, #3625:
URL: https://github.com/apache/dubbo-go/pull/3625

   ### Description
   
   Fixes #3615
   
   **Problem.** With application-level service discovery, a single transient 
`MetadataService` fetch failure during a provider restart could leave the 
consumer directory permanently empty (`No provider available`) until the 
consumer was restarted. Root cause chain, confirmed by a deterministic docker 
reproduction (details in [this 
comment](https://github.com/apache/dubbo-go/issues/3615#issuecomment-5226228151)):
   
   1. `ServiceInstancesChangedListenerImpl.OnEvent` skipped instances whose 
metadata fetch failed, overwrote the service URLs with the (possibly empty) 
result, and returned success. No retry queue, no error propagation — and since 
Nacos only pushes on instance-list *changes*, no later event ever retried the 
fetch.
   2. Even when a later push did re-fetch metadata successfully, 
`RegistryDirectory` refused to rebuild the invoker: the graceful shutdown of 
the provider had left a **closing tombstone** on the instance key (default TTL 
30s), and a genuine same-address restart was indistinguishable from a stale 
pre-shutdown registry snapshot.
   
   **Fix (two layers, both required — verified by e2e):**
   
   - **Metadata retry** (`service_instances_changed_listener_impl.go`): failed 
revisions are tracked in `unresolvedRevisions` and re-resolved by replaying the 
latest instance snapshot through the existing build path, using one shared 
timer per listener with capped exponential backoff (1s→30s, 25% jitter, 
**unlimited attempts** — a retry cap would re-introduce the permanent failure 
this fixes). Retries stop naturally when instances leave the snapshot, when the 
last subscriber is removed, or on registry `Destroy()` 
(`service_discovery_registry.go`). Metadata RPCs no longer run under the 
listener state mutex (build is serialized by a separate `buildMu`; the state 
mutex only guards field access), so a slow/dead provider cannot block event 
processing.
   - **Tombstone restart discrimination** (`directory.go`): the closing 
tombstone now records the closing invoker's export `timestamp`. A re-add 
carrying the *same* timestamp is a stale pre-shutdown snapshot and stays vetoed 
(existing graceful-shutdown semantics unchanged); a *different* timestamp 
proves a genuine restart, so the tombstone is cleared and the invoker rebuilt 
immediately. URLs without a timestamp (older providers) keep the conservative 
veto.
   
   **Known limitation:** export `timestamp` is second-precision in dubbo-go 
(`server/action.go`), so a stop+restart+export+register completing within the 
same second would still be vetoed (degrading to pre-fix behavior until the next 
registry event). This is practically unreachable — graceful stop alone takes 
seconds. If it ever matters, the principled follow-up is discriminating by 
metadata `revision` or aligning the timestamp to milliseconds (Java Dubbo uses 
`System.currentTimeMillis()`), which would be a separate, framework-wide 
discussion.
   
   **Out of scope (tracked separately):** application-level `AddListener` 
failure is likewise never retried — same recovery-gap family, different layer; 
see #3624. Observability for unresolved revisions (e.g. a gauge) belongs to 
#3356.
   
   **Verification:**
   
   - New unit tests (`metadata_retry_test.go`, `directory_test.go`): recovery 
without any further registry event; retries stop when the instance is removed; 
only the latest revision is retried; one shared timer across repeated events; 
timer cancelled on subscriber detach and resumed on re-attach; tombstone vetoes 
stale re-adds but allows genuine restarts.
   - `go test ./registry/...` and the listener/directory tests with `-race` 
pass; broader regression over `registry/metadata/cluster/server/client/common` 
passes.
   - E2E (docker compose: Nacos 2.5.1 + single Triple provider + consumer, 
application-level discovery, local metadata): with traffic to the provider 
blocked during its restart, the consumer failed metadata fetches as before, but 
**self-healed seconds after unblocking without a consumer restart**, followed 
by a 30s steady-state check with all calls succeeding. The same scenario on 
v3.3.2 stays at `No provider available` permanently.
   
   ### 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]

Reply via email to