mochengqian commented on code in PR #970:
URL: https://github.com/apache/dubbo-go-pixiu/pull/970#discussion_r3371911461
##########
pkg/cluster/loadbalancer/load_balancer.go:
##########
@@ -180,26 +194,53 @@ func pickEndpoint(balancer LoadBalancer, context
PickContext, policy model.LbPol
if cursorAfter != cursorBefore {
atomic.AddUint32(&context.Config.PrePickEndpointIndex,
cursorAfter-cursorBefore)
}
- return healthyEndpointFromSnapshot(endpoint, context.HealthyEndpoints)
+ return healthyEndpointFromSnapshot(endpoint, context)
}
func defensiveSnapshotPickContext(context PickContext) PickContext {
defensive := context
defensive.AllEndpoints = model.CloneEndpoints(context.AllEndpoints)
defensive.HealthyEndpoints =
model.CloneEndpoints(context.HealthyEndpoints)
+ defensive.HealthyByID = nil
return defensive
}
-func healthyEndpointFromSnapshot(endpoint *model.Endpoint, healthyEndpoints
[]*model.Endpoint) *model.Endpoint {
+// healthyEndpointFromSnapshot validates that the balancer's pick is still a
+// member of the snapshot's healthy set and returns a defensive clone of the
+// snapshot-owned endpoint (never the balancer's possibly-cloned return).
+//
+// Resolution order:
+//
+// 1. O(1) by-ID recheck: when the pick carries an ID and the context exposes
+// the snapshot's healthy-by-ID index, resolve the single indexed candidate
+// and apply sameEndpointIdentity to it. Every snapshot endpoint carries a
+// unique non-empty ID, so for an ID-bearing pick this is exactly the
+// candidate the scan below would have found — without the O(N) walk
+// (including the per-element SocketAddress.Equal in sameEndpointIdentity).
+// 2. Fallback scan: used when there is no ID index (e.g. a hand-built
context)
+// or the pick has no ID. Keeps the pointer-equality fast path for
zero-copy
+// balancers and the sameEndpointIdentity slow path (including the
+// empty-ID / placeholder-address rules) unchanged.
+//
+// Returns nil when the pick is not present in the healthy set; the validation
+// is deliberate and must not be skipped by blindly cloning the balancer
return.
+func healthyEndpointFromSnapshot(endpoint *model.Endpoint, context
PickContext) *model.Endpoint {
if endpoint == nil {
return nil
}
- for _, candidate := range healthyEndpoints {
+ if endpoint.ID != "" && context.HealthyByID != nil {
+ candidate :=
context.HealthyByID.HealthyEndpointByIDForPick(endpoint.ID)
+ if candidate != nil && sameEndpointIdentity(candidate,
endpoint) {
+ return model.CloneEndpoint(candidate)
+ }
+ return nil
Review Comment:
Addressed in 1e0e07f8: the HealthyByID recheck now returns the cloned
snapshot endpoint immediately when the indexed candidate pointer is the
balancer return (`candidate == endpoint`), preserving the zero-copy pointer
fast path before falling back to `sameEndpointIdentity`. Added
`TestHealthyEndpointFromSnapshotByIDPointerFastPathReturnsClone` to cover the
by-ID pointer path.
--
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]
