Aias00 opened a new issue, #3513:
URL: https://github.com/apache/dubbo-go/issues/3513
### Problem
`base.BaseClusterInvoker.doSelectInvoker` calls
`selectedInvoker.IsAvailable()` immediately after `lb.Select(invokers,
invocation)` without a nil check. Several load-balance implementations can
return `nil` for a non-empty invoker list, so the cluster layer panics on a
nil-pointer dereference.
Affected `lb.Select` implementations that return `nil`:
- **P2C** (`cluster/loadbalance/p2c/loadbalance.go:128,139`) returns `nil`
when `metrics.GetMethodMetrics` returns a non-`ErrMetricsNotFound` error (a
transient metrics-layer error). P2C also `panic(...)` outright when the metrics
value is not `uint64` (~line 145).
- **ConsistentHash**
(`cluster/loadbalance/consistenthashing/loadbalance.go:71`) returns `nil` when
`json.Marshal(invoker)` fails.
The `adaptivesvc` fast path forces P2C and has the same unguarded
dereference (`cluster/cluster/adaptivesvc/cluster_invoker.go:66-70`).
### Current behavior
`cluster/cluster/base/cluster_invoker.go:160-163`
```go
selectedInvoker := lb.Select(invokers, invocation)
// judge if the selected Invoker is invoked and available
if (!selectedInvoker.IsAvailable() && invoker.AvailableCheck) ||
isInvoked(selectedInvoker, invoked) {
```
`cluster/cluster/adaptivesvc/cluster_invoker.go:66-70`
```go
lb := extension.GetLoadbalance(lbKey) // forced to P2C
invoker := lb.Select(invokers, invocation) // can return nil
...
res := invoker.Invoke(ctx, invocation) // nil dereference → panic
```
`cluster/loadbalance/p2c/loadbalance.go:128,139,~145`
```go
if err != nil {
if errors.Is(err, metrics.ErrMetricsNotFound) {
return invokers[i]
}
logger.Warnf(...)
return nil
}
...
remainingI, ok := remainingIIface.(uint64)
if !ok {
panic(fmt.Sprintf("[Loadbalance][P2C] type check failed: ..."))
}
```
### Expected behavior
1. `lb.Select` should have a clear contract for non-empty input: either
never return `nil`, or callers must handle `nil` by degrading to a fallback
selection.
2. `doSelectInvoker` and the `adaptivesvc` fast path must nil-check before
dereferencing.
3. P2C must not `panic` on an unexpected metrics type; it should degrade and
warn.
### Suggested approach
- Add `if selectedInvoker == nil { /* degrade to first available / random */
}` in `doSelectInvoker` and in the `adaptivesvc` fast path.
- Tighten the `LoadBalance.Select` contract (return `(Invoker, error)`, or
guarantee non-nil for non-empty input) and audit all implementations.
- Replace P2C's `panic(...)` on type mismatch with a warning + degraded
selection.
### Acceptance criteria
- [ ] P2C transient metrics error no longer causes a panic in the cluster
layer; the call degrades to an available invoker.
- [ ] ConsistentHash `json.Marshal` failure no longer causes a nil-deref in
`doSelectInvoker`.
- [ ] `adaptivesvc` fast path nil-checks the P2C result before `Invoke`.
- [ ] P2C no longer `panic`s on a non-`uint64` metrics value.
- [ ] Regression tests cover P2C transient error and ConsistentHash marshal
failure paths (`-race` clean).
--
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]
