mochengqian opened a new issue, #957:
URL: https://github.com/apache/dubbo-go-pixiu/issues/957
## Background
Follow-up to #932 (issue #905 step 5). Distinct from #942 — see
"Relationship to #942" below.
After #932, snapshot-aware consistent-hash balancers (`RingHashing`,
`MaglevHash`) read `EndpointSnapshot.HealthyConsistentHash` (lazily built from
the healthy subset). But `ClusterStore.prepareClusterConfig` still eagerly
rebuilds `Config.ConsistentHash.Hash` on every config mutation:
```go
func (s *ClusterStore) prepareClusterConfig(c *model.ClusterConfig) {
s.assembleClusterEndpoints(c)
c.CreateConsistentHash()
}
```
`prepareClusterConfig` runs on `AddCluster`, `UpdateCluster`, `SetEndpoint`,
and `DeleteEndpoint`.
## Problem
`Config.ConsistentHash.Hash` is now only consumed by the **legacy**
(non-snapshot) balancer path — all bundled balancers are snapshot-aware and use
the snapshot's own healthy hash. So for the common path this rebuild is dead
work. For a Maglev cluster with a large table (e.g. `MaglevTableSize` in the
tens of thousands), rebuilding on every endpoint add/remove is milliseconds of
wasted CPU, and high-churn service-discovery environments (Nacos instances
flapping) trigger it repeatedly.
## Goal
Stop rebuilding `Config.ConsistentHash.Hash` eagerly. Build it lazily, only
when a legacy balancer first needs it.
## Suggested approach
1. Remove `c.CreateConsistentHash()` from `prepareClusterConfig`.
2. Add a lazy guard on `ClusterConfig`, e.g. `consistentHashOnce sync.Once`
(tagged `yaml:"-" json:"-"`). **Must be a value, not a pointer** —
`ClusterConfig` is deep-copied via `CloneStore` (yaml round-trip), and a
pointer `Once` would be shared across stores. A fresh store should rebuild
independently.
3. In the legacy branch of `pickEndpoint`
(`pkg/cluster/loadbalancer/load_balancer.go`), trigger
`context.Config.consistentHashOnce.Do(context.Config.CreateConsistentHash)`
before using the hash.
4. The existing in-`Handler` nil fallback in `Maglev.Handler` /
`RingHashing.Handler` (build-on-the-fly when `Hash == nil`) stays as a safety
net.
## Relationship to #942
These are complementary, not duplicates:
- **#942** reuses the *snapshot's* `consistentHash` across publications when
the healthy set is unchanged (avoids rebuild on health flaps inside the
snapshot path).
- **This issue** stops `prepareClusterConfig` from eagerly building the
*Config-level* `ConsistentHash.Hash` that only the legacy path reads.
Different fields, different code paths, different triggers.
## Risk
If any code reads `c.ConsistentHash.Hash` directly *before* a balancer
`Handler` runs, it now sees `nil`. `grep -rn "ConsistentHash\.Hash" pkg/` to
confirm; currently only `maglev_hash.go` and `ring_hash.go` read it, both
inside `Handler`. Any external/xDS reader must be reviewed.
## Test plan
- [ ] New test: repeated `SetEndpoint` triggers zero `CreateConsistentHash`
calls on the snapshot path (mock `ConsistentHashInitFunc` and count
invocations).
- [ ] Legacy `Handler` builds the hash exactly once via the `sync.Once`.
- [ ] `BenchmarkSetEndpoint` on a large-Maglev cluster, before/after.
- [ ] `go test -race ./pkg/model/... ./pkg/server/...
./pkg/cluster/loadbalancer/...`
## Files
- `pkg/server/cluster_manager.go`
- `pkg/model/cluster.go`
- `pkg/cluster/loadbalancer/load_balancer.go`
--
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]
