Copilot commented on code in PR #970:
URL: https://github.com/apache/dubbo-go-pixiu/pull/970#discussion_r3371156037
##########
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:
With HealthyByID set (production request path) and non-empty endpoint.ID,
the O(1) branch bypasses the pointer-equality fast path entirely and always
calls sameEndpointIdentity. For zero-copy balancers this adds unnecessary
SocketAddress.Equal work; you can keep the fastest path by short-circuiting
when the indexed candidate pointer equals the balancer return (candidate ==
endpoint) before running sameEndpointIdentity.
--
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]
