ywxzm03 opened a new pull request, #961:
URL: https://github.com/apache/dubbo-go-pixiu/pull/961
**What this PR does**:
This PR reduces duplicated endpoint deep clones in the `SetEndpoint`
membership churn path, while preserving the existing ownership safety
boundaries.
Specifically, this PR:
- Keeps the snapshot-side clone.
- Separates endpoint ID/name defaulting from the endpoint clone boundary.
- For endpoints already owned by `ClusterStore` in the `SetEndpoint` /
`DeleteEndpoint` paths, this PR uses a store-owned prepare path to avoid
another full `CloneEndpoints`.
- It keeps the store-side clone at external input boundaries, so Pixiu
still does not mutate endpoint objects provided by callers.
- Adds a membership churn benchmark that measures repeated `SetEndpoint`
metadata updates with 1000 endpoints.
This PR strictly follows these constraints:
- Do not change endpoint ID generation.
- Do not mutate user-supplied endpoint objects after `AddCluster` or
`SetEndpoint`.
- Do not share snapshot-owned endpoint pointers with code that may mutate
them.
- Do not rewrite the snapshot mechanism.
**Which issue(s) this PR fixes**:
Fixes #940
**Special notes for your reviewer**:
This PR chooses the second approach suggested in the issue:
```text
Keep snapshot-side clone, move ID/name defaulting to a point where
caller-owned objects are not mutated.
```
It does not choose the first approach:
```text
Keep store-side clone, make snapshot publication reuse safe already-owned
endpoint objects if possible.
```
Why the first approach was not chosen:
Keeping the store-side clone and allowing snapshots to reuse store-owned
endpoints can reduce one deep clone. However, `Cluster.Config` is still a
publicly mutable field today. If a snapshot reuses endpoint pointers from
`Config.Endpoints`, later direct mutations to `Cluster.Config.Endpoints`,
`Metadata`, `LLMMeta`, or `UnHealthy` could pollute an already published
runtime snapshot.
To implement the first approach safely, we would likely need changes such as:
- making `Cluster.Config` private and exposing it through read-only or
clone-returning getters;
- introducing truly immutable/frozen endpoints;
- splitting the public config from the snapshot-owned endpoint source;
- auditing all config mutation paths and making them copy-on-write.
That scope is much larger and gets close to the “Do not rewrite the snapshot
mechanism” boundary. Therefore, this PR does not choose the first approach.
Instead, it keeps the snapshot-side clone and only removes the duplicated clone
from store-owned mutation paths.
**Verification**:
```bash
go test ./pkg/server/... ./pkg/cluster ./pkg/model/...
go test -run '^$' -bench '^BenchmarkClusterSetEndpointMembershipChurn$'
-benchmem ./pkg/server
```
Local benchmark results before and after this change:
Before:
```text
goos: darwin
goarch: arm64
pkg: github.com/apache/dubbo-go-pixiu/pkg/server
cpu: Apple M5
BenchmarkClusterSetEndpointMembershipChurn-10 2100
478004 ns/op 1435526 B/op 9060 allocs/op
PASS
ok github.com/apache/dubbo-go-pixiu/pkg/server 2.198s
```
After:
```text
goos: darwin
goarch: arm64
pkg: github.com/apache/dubbo-go-pixiu/pkg/server
cpu: Apple M5
BenchmarkClusterSetEndpointMembershipChurn-10 2830
358501 ns/op 947546 B/op 6061 allocs/op
PASS
ok github.com/apache/dubbo-go-pixiu/pkg/server 1.562s
```
**Does this PR introduce a user-facing change?**
```release-note
NONE
```
--
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]
