Copilot commented on code in PR #973:
URL: https://github.com/apache/dubbo-go-pixiu/pull/973#discussion_r3379736634
##########
pkg/cluster/loadbalancer/load_balancer.go:
##########
@@ -173,10 +176,15 @@ func pickEndpoint(balancer LoadBalancer, context
PickContext, policy model.LbPol
}
config := *context.Config
config.Endpoints = model.CloneEndpoints(allEndpoints)
- cursorBefore := atomic.LoadUint32(&context.Config.PrePickEndpointIndex)
+ var cursorBefore, cursorAfter uint32
+ if context.RoundRobinCursor != nil {
+ cursorBefore = context.RoundRobinCursor.Load()
+ } else {
+ cursorBefore =
atomic.LoadUint32(&context.Config.PrePickEndpointIndex)
+ }
atomic.StoreUint32(&config.PrePickEndpointIndex, cursorBefore)
endpoint := balancer.Handler(&config, policy)
- cursorAfter := atomic.LoadUint32(&config.PrePickEndpointIndex)
+ cursorAfter = atomic.LoadUint32(&config.PrePickEndpointIndex)
if cursorAfter != cursorBefore {
atomic.AddUint32(&context.Config.PrePickEndpointIndex,
cursorAfter-cursorBefore)
}
Review Comment:
When RoundRobinCursor is provided (which ClusterManager now always sets),
the legacy balancer reconciliation updates context.Config.PrePickEndpointIndex
instead of advancing the runtime cursor. That means legacy balancers that rely
on PrePickEndpointIndex (including custom/out-of-tree ones) will observe a
fixed cursor and can keep picking the same slot repeatedly. The cursor delta
should be applied back to RoundRobinCursor when it is present.
##########
pkg/cluster/cluster.go:
##########
@@ -49,16 +62,18 @@ func NewCluster(clusterConfig *model.ClusterConfig)
*Cluster {
func NewClusterWithEndpointSnapshot(clusterConfig *model.ClusterConfig,
previous *EndpointSnapshot) *Cluster {
c := &Cluster{
- Config: clusterConfig,
+ config: clusterConfig,
+ configID: clusterConfig.ConfigID,
+ runtimeState: &RuntimeState{},
acceptHealthEvents: true,
}
c.RefreshEndpointsFrom(previous)
// only handle one health checker
- if len(c.Config.HealthChecks) != 0 {
+ if len(c.config.HealthChecks) != 0 {
c.HealthCheck = healthcheck.CreateHealthCheckWithCallback(
clusterConfig,
- c.Config.HealthChecks[0],
+ c.config.HealthChecks[0],
c.handleEndpointHealth,
)
c.HealthCheck.Start()
Review Comment:
NewClusterWithEndpointSnapshot now dereferences clusterConfig (ConfigID and
HealthChecks) without a nil guard, which can panic if a nil config is ever
passed (previously the type largely tolerated a nil config and built an empty
snapshot). Adding nil checks keeps the constructor robust and preserves prior
behavior.
--
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]
