mochengqian opened a new pull request, #970: URL: https://github.com/apache/dubbo-go-pixiu/pull/970
## Summary Closes #955. The request-path pick verifies a balancer's returned endpoint is still in the snapshot's healthy set via `healthyEndpointFromSnapshot`. It scanned the healthy slice twice — a pointer-equality pass, then a `sameEndpointIdentity` pass (with a per-element `SocketAddress.Equal`). Zero-copy balancers hit the pointer pass early, but **non-zero-copy** snapshot balancers go through `defensiveSnapshotPickContext`, which clones `HealthyEndpoints`; the balancer returns an element of the *cloned* slice, so its pointer never equals any element of `context.HealthyEndpoints` and every pick fell through to the full O(N) identity scan. This PR makes the recheck O(1) without weakening the trust-boundary validation #932 intentionally keeps. ## Change - `EndpointSnapshot.HealthyEndpointByIDForPick(id)` — exposes the existing `healthyEndpointByID` index as an O(1), allocation-free, snapshot-owned (no-clone) lookup, mirroring the `*ForPick` no-mutate/no-retain contract. - `PickContext.HealthyByID` (new field) typed as a small `HealthyEndpointByIDLookup` interface declared in `loadbalancer`, so the package keeps **no dependency** on `pkg/cluster`. `*cluster.EndpointSnapshot` satisfies it; `cluster_manager` wires the live snapshot in. - `healthyEndpointFromSnapshot` now resolves the single candidate by ID and applies the **unchanged** `sameEndpointIdentity` rules to it. It falls back to the original two-pass scan when there is no index (hand-built contexts) or the pick has no ID — preserving the pointer-equality fast path, the blank-domain placeholder wildcard (rule 2), and the reject-on-not-found defense. Every snapshot endpoint carries a unique non-empty ID, so for an ID-bearing pick the by-ID candidate is exactly the one the scan would have found. ## Correctness `sameEndpointIdentity` semantics are unchanged — the optimization only changes *how* the candidate is located, not the accept/reject rule applied to it. A new table test (`TestHealthyEndpointFromSnapshotByIDMatchesScan`) asserts the by-ID path and the scan path return the identical decision for: defensive-copy match, placeholder wildcard, same-ID/different-address rejection, and unknown-ID rejection. The existing placeholder/mismatch/pointer-clone tests and the "rejects unhealthy snapshot/legacy return" integration tests still pass. ## Benchmark `BenchmarkHealthyEndpointFromSnapshot`, 1024 healthy endpoints, non-zero-copy return with the ID at the far end of the slice: | | ns/op | B/op | allocs/op | |---|---|---|---| | identity-scan-without-index (old behavior) | 1368 | 144 | 1 | | identity-recheck-with-index (this PR) | 36 | 144 | 1 | ~38× faster on the worst case. The 1 alloc / 144 B is the result `CloneEndpoint`, unchanged across both — as the issue's caveat predicted, clone cost is fixed, so the win is the removed O(N) walk. The pointer-eq fast path for zero-copy balancers is unaffected (measured: no regression). ## Test plan - [x] `go test -race ./pkg/cluster/... ./pkg/cluster/loadbalancer/... ./pkg/server/...` - [x] `go vet` clean on all three packages - [x] new `HealthyEndpointByIDForPick` test (no-clone identity, unhealthy/missing/nil-safe) - [x] new by-ID-matches-scan equivalence test - [x] benchmark documents before/after across zero-copy and non-zero-copy paths - [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]
