mochengqian opened a new pull request, #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971
## Summary
Closes #941.
The zero-copy and healthy-only snapshot fast paths were opt-in via two
**exported** marker interfaces, `ZeroCopySnapshotLoadBalancer` and
`HealthyOnlySnapshotLoadBalancer`. Because Go interface satisfaction is
structural, any external load-balancer plugin could implement those method
names and silently opt into contracts that require never mutating or retaining
snapshot-owned endpoints — risking request-path mutation, stale pointer
retention, or data races on the shared immutable snapshot.
This PR replaces the two markers with a single **internal** opt-in that
external plugins cannot satisfy.
## Mechanism
- New package `pkg/cluster/loadbalancer/internal/snapshotopt` defines
`Token{ZeroCopy, HealthyOnly bool}`.
- Balancers opt in via `SnapshotOptIn() snapshotopt.Token`. The return type
lives under `internal/`, so only packages rooted at `pkg/cluster/loadbalancer`
can import it and name the type — i.e. only trusted in-tree balancers can
implement the opt-in interface.
- External plugins **cannot construct a `Token`**, cannot satisfy the
interface, and so always fall through to the safe default: full snapshot,
defensively copied.
This is strictly stronger than relocating the interfaces would be: because
Go interfaces are structural, an external type with the same method names would
still match a relocated interface. Returning an unnameable type closes that
hole.
## Scope
- Public extension surface (`SnapshotLoadBalancer`, `PickContext`) unchanged.
- The two exported marker interfaces are removed; `NeedsAllEndpoints` and
the zero-copy branch in `pickEndpoint` now read the token via an internal
`snapshotOptIn` helper.
- All five bundled balancers (RoundRobin, Rand, WeightRandom, Maglev,
RingHash) converted from the two `Use*` methods to one `SnapshotOptIn`
returning `{ZeroCopy: true, HealthyOnly: true}` — behavior identical.
- Updated the stale `ZeroCopySnapshotLoadBalancer` reference in the
`cluster.go` snapshot-sharing doc comment.
## Tests
- New `TestExternalLikeBalancerCannotOptIntoFastPaths`: a balancer
reproducing the old `UseZeroCopySnapshot`/`UseHealthyEndpointsOnly` method
names is treated as **untrusted** — `NeedsAllEndpoints` returns true (full
snapshot) and its mutation does not escape (defensive copy). This is the
trust-boundary regression the issue asks for.
- Existing `TestPickEndpointSnapshotMutationsDoNotEscape` (unmarked balancer
→ defensive copy) and `TestNeedsAllEndpoints...` still pass with the
healthy-only test double converted to the token.
## Test plan
- [x] `go test -race ./pkg/cluster/... ./pkg/server/...`
- [x] `go vet ./pkg/cluster/...` clean
- [x] `go build ./...` clean
- [x] pre-push local-CI passed
--
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]
