Copilot commented on code in PR #925:
URL: https://github.com/apache/dubbo-go-pixiu/pull/925#discussion_r3212586418
##########
pkg/cluster/loadbalancer/load_balancer.go:
##########
@@ -17,24 +17,83 @@
package loadbalancer
+import (
+ "sync"
+ "sync/atomic"
+)
+
import (
"github.com/apache/dubbo-go-pixiu/pkg/model"
)
+type PickContext struct {
+ // Config carries cluster-level load-balancer configuration and runtime
+ // cursor state. Snapshot-aware balancers should not reread
Config.Endpoints
+ // for health filtering.
+ Config *model.ClusterConfig
+ // HealthyEndpoints is already filtered from the current runtime
snapshot.
+ HealthyEndpoints []*model.Endpoint
+}
+
type LoadBalancer interface {
Handler(c *model.ClusterConfig, policy model.LbPolicy) *model.Endpoint
}
+// SnapshotLoadBalancer is optional for balancers that consume the current
+// runtime snapshot. Implementations should pick only from
+// PickContext.HealthyEndpoints; Config.Endpoints may include unhealthy or
stale
+// config entries.
+type SnapshotLoadBalancer interface {
+ HandlerWithSnapshot(c PickContext, policy model.LbPolicy)
*model.Endpoint
+}
+
// LoadBalancerStrategy load balancer strategy mode
var LoadBalancerStrategy = map[model.LbPolicyType]LoadBalancer{}
+var legacyPickMu sync.Mutex
+
func RegisterLoadBalancer(name model.LbPolicyType, balancer LoadBalancer) {
if _, ok := LoadBalancerStrategy[name]; ok {
panic("load balancer register fail " + name)
}
LoadBalancerStrategy[name] = balancer
}
+func PickEndpoint(balancer LoadBalancer, context PickContext, policy
model.LbPolicy) *model.Endpoint {
+ if balancer == nil || context.Config == nil {
+ return nil
+ }
+ if snapshotBalancer, ok := balancer.(SnapshotLoadBalancer); ok {
+ return snapshotBalancer.HandlerWithSnapshot(context, policy)
+ }
+
+ // Legacy balancers only understand ClusterConfig. Serialize this
+ // compatibility path so cursor-style state reconciles predictably;
custom
+ // mutable state should move to SnapshotLoadBalancer instead.
+ legacyPickMu.Lock()
+ defer legacyPickMu.Unlock()
+
Review Comment:
`legacyPickMu` is a single global mutex, so any cluster using a legacy
(non-SnapshotLoadBalancer) implementation will serialize *all* legacy picks
across the entire process (including unrelated clusters). This can become a
cross-cluster throughput bottleneck under load. Consider scoping the
serialization to the specific cluster (e.g., a
per-ClusterConfig/per-runtime-cluster mutex, or a keyed mutex map) so different
clusters can pick concurrently while still preserving cursor/state
reconciliation for a given cluster.
--
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]
