DadaVinqi opened a new pull request, #3599:
URL: https://github.com/apache/dubbo-go/pull/3599
## Summary
This PR fixes three shutdown and cancellation issues tracked by #3555:
- Failback retry workers may outlive the invoker lifecycle.
- `BaseRegistry.Subscribe` cannot be interrupted while waiting between
retries.
- Getty connection establishment may block `Close()` until the full
connection timeout.
No public API signatures are changed.
Fixes #3555
## Problem
During shutdown, several background loops do not respond promptly to the
component lifecycle:
1. Failback retry processing can continue after the invoker has been
destroyed.
2. Registry subscription retries use an uninterruptible `time.Sleep`, so
`Destroy()` may leave `Subscribe()` blocked for the full retry delay.
3. Getty connection establishment waits in a polling loop while holding a
read lock required by `Client.Close()`. As a result, `Close()` cannot publish
the closed state until connection establishment finishes.
These behaviors can delay graceful shutdown, leave retry work running
against destroyed components, and retain resources longer than expected.
## Root Cause
### Failback
Failback retries were not tied to an invoker-owned lifecycle context. The
retry processor and retry goroutines did not have a common cancellation path
coordinated by `Destroy()`, and enqueueing new retry tasks was not fully
guarded after destruction.
### BaseRegistry
`BaseRegistry.Subscribe` used:
```go
time.Sleep(time.Duration(RegistryConnDelay) * time.Second)
```
The sleep could not observe `BaseRegistry.Done()`. In addition, the
subscription loop did not check registry availability after `listener.Next()`
returned, so an event could still be delivered during a shutdown race.
### Getty
`selectSession` held the client read lock while calling
`newGettyRPCClientConn`, including its connection wait loop. `Client.Close()`
requires the corresponding write lock, so it could not mark the client closed
while connection establishment was in progress.
The connection wait loop also used `time.Sleep`, which could not be
interrupted immediately by client shutdown.
## Solution
### Failback lifecycle cancellation
- Added a retry lifecycle context owned by the failback invoker.
- Created the context with `context.WithCancel(context.WithoutCancel(...))`
so retries retain the intended lifecycle behavior while remaining cancellable
by the invoker.
- Added cancellation checks to the retry processor and retry execution path.
- Made `Destroy()` idempotent.
- `Destroy()` now:
- Marks the invoker stopped.
- Cancels the retry context.
- Disposes the retry queue.
- Waits for the retry processor and active retry tasks with a bounded
timeout.
- Delegates to the underlying invoker destroy logic.
- Prevented new retry tasks from being enqueued after destruction.
- Prevented invocation and retry processing from continuing after shutdown.
### Interruptible Registry subscription retry
- Replaced the retry `time.Sleep` with a timer selecting on:
- The retry timer.
- `BaseRegistry.Done()`.
- Added availability checks before and after `listener.Next()`.
- Close the listener and return the existing unavailable error when
destruction is observed.
- Kept the public `Subscribe` API unchanged.
- Did not modify provider-specific listener implementations.
### Getty connection shutdown
- Replaced the mutable `clientClosed` flag with an atomic closed state.
- Added a client shutdown channel that is closed immediately by
`Client.Close()`.
- Added a dedicated connection mutex to serialize slow connection creation.
- Restricted `gettyClientMux` to protecting the published Getty connection
pointer.
- Removed the long-held client read lock around connection establishment.
- Added double checks before and after connection creation so a connection
cannot be published after shutdown.
- Close an unpublished connection if shutdown occurs during creation.
- Changed connection reset logic to clear only the expected connection,
preventing an old connection from clearing a newer one.
- Replaced the Getty polling sleep with an interruptible timer wait.
- Preserved the existing connection timeout behavior.
## Regression Tests
Added coverage for:
- Failback cancellation during retry execution.
- Idempotent failback destruction.
- Bounded failback shutdown waiting.
- Preventing retry enqueue after destruction.
- Registry retry cancellation during `Destroy()`.
- Preventing registry notifications after destruction.
- Getty connection wait cancellation.
- Getty timeout behavior.
- `Client.Close()` not waiting for the connection mutex.
- Closing a Getty connection creation attempt in progress.
## Test Plan
- [x] `make test`
- [x] `make lint`
- [x] `make fmt`
- [x] `go test -count=1 ./cluster/... ./registry/... ./remoting/...`
- [x] `go test -count=1 -race ./registry ./cluster/cluster/failback`
- [x] Focused Getty shutdown tests with `-race`
- [x] `go vet`
- [x] `git diff --check`
--
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]
