You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "mochengqian (via GitHub)" <gi...@apache.org> on 2026/06/07 17:20:27 UTC
[PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
mochengqian opened a new pull request, #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971
## Summary
Closes #941.
The zero-copy and healthy-only snapshot fast paths were opt-in via two **exported** marker interfaces, `ZeroCopySnapshotLoadBalancer` and `HealthyOnlySnapshotLoadBalancer`. Because Go interface satisfaction is structural, any external load-balancer plugin could implement those method names and silently opt into contracts that require never mutating or retaining snapshot-owned endpoints — risking request-path mutation, stale pointer retention, or data races on the shared immutable snapshot.
This PR replaces the two markers with a single **internal** opt-in that external plugins cannot satisfy.
## Mechanism
- New package `pkg/cluster/loadbalancer/internal/snapshotopt` defines `Token{ZeroCopy, HealthyOnly bool}`.
- Balancers opt in via `SnapshotOptIn() snapshotopt.Token`. The return type lives under `internal/`, so only packages rooted at `pkg/cluster/loadbalancer` can import it and name the type — i.e. only trusted in-tree balancers can implement the opt-in interface.
- External plugins **cannot construct a `Token`**, cannot satisfy the interface, and so always fall through to the safe default: full snapshot, defensively copied.
This is strictly stronger than relocating the interfaces would be: because Go interfaces are structural, an external type with the same method names would still match a relocated interface. Returning an unnameable type closes that hole.
## Scope
- Public extension surface (`SnapshotLoadBalancer`, `PickContext`) unchanged.
- The two exported marker interfaces are removed; `NeedsAllEndpoints` and the zero-copy branch in `pickEndpoint` now read the token via an internal `snapshotOptIn` helper.
- All five bundled balancers (RoundRobin, Rand, WeightRandom, Maglev, RingHash) converted from the two `Use*` methods to one `SnapshotOptIn` returning `{ZeroCopy: true, HealthyOnly: true}` — behavior identical.
- Updated the stale `ZeroCopySnapshotLoadBalancer` reference in the `cluster.go` snapshot-sharing doc comment.
## Tests
- New `TestExternalLikeBalancerCannotOptIntoFastPaths`: a balancer reproducing the old `UseZeroCopySnapshot`/`UseHealthyEndpointsOnly` method names is treated as **untrusted** — `NeedsAllEndpoints` returns true (full snapshot) and its mutation does not escape (defensive copy). This is the trust-boundary regression the issue asks for.
- Existing `TestPickEndpointSnapshotMutationsDoNotEscape` (unmarked balancer → defensive copy) and `TestNeedsAllEndpoints...` still pass with the healthy-only test double converted to the token.
## Test plan
- [x] `go test -race ./pkg/cluster/... ./pkg/server/...`
- [x] `go vet ./pkg/cluster/...` clean
- [x] `go build ./...` clean
- [x] pre-push local-CI passed
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "mochengqian (via GitHub)" <gi...@apache.org>.
mochengqian commented on code in PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#discussion_r3371967118
##########
pkg/cluster/loadbalancer/load_balancer.go:
##########
@@ -63,20 +64,45 @@ type SnapshotLoadBalancer interface {
HandlerWithSnapshot(c PickContext, policy model.LbPolicy) *model.Endpoint
}
-// HealthyOnlySnapshotLoadBalancer marks snapshot-aware balancers that do not
-// need PickContext.AllEndpoints. Unmarked snapshot balancers keep receiving
-// the full snapshot for compatibility with custom implementations.
+// HealthyOnlySnapshotLoadBalancer is retained for source compatibility with
+// callers that referenced the old marker contract.
+//
+// Deprecated: runtime fast-path decisions ignore this interface. Only trusted
+// in-tree balancers can opt in via SnapshotOptIn returning snapshotopt.Token.
type HealthyOnlySnapshotLoadBalancer interface {
UseHealthyEndpointsOnly() bool
}
-// ZeroCopySnapshotLoadBalancer marks trusted balancers that never mutate or
-// retain snapshot endpoints. Other snapshot balancers receive defensive
-// copies.
+// ZeroCopySnapshotLoadBalancer is retained for source compatibility with
+// callers that referenced the old marker contract.
+//
+// Deprecated: runtime fast-path decisions ignore this interface. Only trusted
+// in-tree balancers can opt in via SnapshotOptIn returning snapshotopt.Token.
type ZeroCopySnapshotLoadBalancer interface {
UseZeroCopySnapshot() bool
}
+// snapshotOptInBalancer is the internal opt-in surface for trusted, in-tree
+// snapshot balancers. The method returns snapshotopt.Token, whose type lives in
+// an internal package, so only balancers under pkg/cluster/loadbalancer can
+// implement this interface. External plugins cannot name the return type and
+// therefore always fall through to the safe default (full snapshot, defensively
+// copied). This is the trust boundary described in issue #941.
+type snapshotOptInBalancer interface {
+ SnapshotOptIn() snapshotopt.Token
+}
Review Comment:
Good catch — fixed in fceeb80d. `snapshotOptIn` now also runs a runtime trust check: it resolves the concrete balancer's `reflect.Type.PkgPath()` and only honors the token when the type lives under `github.com/apache/dubbo-go-pixiu/pkg/cluster/loadbalancer`. An external type that embeds an in-tree balancer keeps its own package path, so the promoted `SnapshotOptIn` no longer grants the fast paths. Added `TestEmbeddingInTreeBalancerCannotOptIntoFastPaths` in an external `loadbalancer_test` package as the regression — it fails without the check and passes with it.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "Similarityoung (via GitHub)" <gi...@apache.org>.
Similarityoung commented on PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#issuecomment-5082226257
please resolve conflicts
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#issuecomment-4643385483
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=971) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=971&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=971&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=971)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "Similarityoung (via GitHub)" <gi...@apache.org>.
Similarityoung commented on PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#issuecomment-5082223052
please resolve conflicts
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#issuecomment-5087845007
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=971) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=971&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=971&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=971)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "mochengqian (via GitHub)" <gi...@apache.org>.
mochengqian commented on PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#issuecomment-5087860388
> please resolve conflicts
resolved.thank you
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "mochengqian (via GitHub)" <gi...@apache.org>.
mochengqian commented on code in PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#discussion_r3371969876
##########
pkg/cluster/loadbalancer/load_balancer.go:
##########
@@ -63,20 +64,45 @@ type SnapshotLoadBalancer interface {
HandlerWithSnapshot(c PickContext, policy model.LbPolicy) *model.Endpoint
}
-// HealthyOnlySnapshotLoadBalancer marks snapshot-aware balancers that do not
-// need PickContext.AllEndpoints. Unmarked snapshot balancers keep receiving
-// the full snapshot for compatibility with custom implementations.
+// HealthyOnlySnapshotLoadBalancer is retained for source compatibility with
+// callers that referenced the old marker contract.
+//
+// Deprecated: runtime fast-path decisions ignore this interface. Only trusted
+// in-tree balancers can opt in via SnapshotOptIn returning snapshotopt.Token.
type HealthyOnlySnapshotLoadBalancer interface {
UseHealthyEndpointsOnly() bool
}
-// ZeroCopySnapshotLoadBalancer marks trusted balancers that never mutate or
-// retain snapshot endpoints. Other snapshot balancers receive defensive
-// copies.
+// ZeroCopySnapshotLoadBalancer is retained for source compatibility with
+// callers that referenced the old marker contract.
+//
+// Deprecated: runtime fast-path decisions ignore this interface. Only trusted
+// in-tree balancers can opt in via SnapshotOptIn returning snapshotopt.Token.
type ZeroCopySnapshotLoadBalancer interface {
UseZeroCopySnapshot() bool
}
Review Comment:
Resolved by removing the interfaces. The markers were added in #932 and never shipped in a tagged release, so there is no external caller to stay source-compatible with — keeping them as Deprecated only contradicted the PR goal. In fceeb80d both `HealthyOnlySnapshotLoadBalancer` and `ZeroCopySnapshotLoadBalancer` are deleted, and the PR description is updated to match.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "Similarityoung (via GitHub)" <gi...@apache.org>.
Similarityoung merged PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#issuecomment-4643472769
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=971) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=971&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=971&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=971)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#issuecomment-4659238339
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `52.00000%` with `12 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 26.19%. Comparing base ([`d2e421d`](https://app.codecov.io/gh/apache/dubbo-go-pixiu/commit/d2e421d99ee5770798ad82f3f1efe3fcf496035f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`fceeb80`](https://app.codecov.io/gh/apache/dubbo-go-pixiu/commit/fceeb80d6640b4202758ea615bc0a58aa0175aa7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on develop.
| [Files with missing lines](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [pkg/cluster/loadbalancer/load\_balancer.go](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&filepath=pkg%2Fcluster%2Floadbalancer%2Fload_balancer.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2NsdXN0ZXIvbG9hZGJhbGFuY2VyL2xvYWRfYmFsYW5jZXIuZ28=) | 86.66% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pkg/cluster/loadbalancer/maglev/maglev\_hash.go](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&filepath=pkg%2Fcluster%2Floadbalancer%2Fmaglev%2Fmaglev_hash.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2NsdXN0ZXIvbG9hZGJhbGFuY2VyL21hZ2xldi9tYWdsZXZfaGFzaC5nbw==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...kg/cluster/loadbalancer/rand/load\_balancer\_rand.go](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&filepath=pkg%2Fcluster%2Floadbalancer%2Frand%2Fload_balancer_rand.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2NsdXN0ZXIvbG9hZGJhbGFuY2VyL3JhbmQvbG9hZF9iYWxhbmNlcl9yYW5kLmdv) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pkg/cluster/loadbalancer/ringhash/ring\_hash.go](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&filepath=pkg%2Fcluster%2Floadbalancer%2Fringhash%2Fring_hash.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2NsdXN0ZXIvbG9hZGJhbGFuY2VyL3JpbmdoYXNoL3JpbmdfaGFzaC5nbw==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pkg/cluster/loadbalancer/roundrobin/round\_robin.go](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&filepath=pkg%2Fcluster%2Floadbalancer%2Froundrobin%2Fround_robin.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2NsdXN0ZXIvbG9hZGJhbGFuY2VyL3JvdW5kcm9iaW4vcm91bmRfcm9iaW4uZ28=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...cluster/loadbalancer/weightrandom/weight\_random.go](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&filepath=pkg%2Fcluster%2Floadbalancer%2Fweightrandom%2Fweight_random.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2NsdXN0ZXIvbG9hZGJhbGFuY2VyL3dlaWdodHJhbmRvbS93ZWlnaHRfcmFuZG9tLmdv) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #971 +/- ##
===========================================
+ Coverage 26.14% 26.19% +0.04%
===========================================
Files 275 275
Lines 21981 21982 +1
===========================================
+ Hits 5748 5759 +11
+ Misses 15623 15611 -12
- Partials 610 612 +2
```
| [Flag](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.19% <52.00%> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/dubbo-go-pixiu/pull/971?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "mochengqian (via GitHub)" <gi...@apache.org>.
mochengqian commented on code in PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#discussion_r3371973114
##########
pkg/cluster/loadbalancer/roundrobin/round_robin.go:
##########
@@ -32,6 +33,13 @@ func init() {
type RoundRobin struct{}
+var _ loadbalancer.HealthyOnlySnapshotLoadBalancer = RoundRobin{}
+var _ loadbalancer.ZeroCopySnapshotLoadBalancer = RoundRobin{}
Review Comment:
Agreed — done in fceeb80d. Removed the `var _ ...SnapshotLoadBalancer` assertions and the `Use*` methods; the balancer now opts in solely through `SnapshotOptIn()`. The deprecated marker interfaces themselves are also deleted.
##########
pkg/cluster/loadbalancer/rand/load_balancer_rand.go:
##########
@@ -32,6 +33,13 @@ func init() {
type Rand struct{}
+var _ loadbalancer.HealthyOnlySnapshotLoadBalancer = Rand{}
+var _ loadbalancer.ZeroCopySnapshotLoadBalancer = Rand{}
Review Comment:
Agreed — done in fceeb80d. Removed the `var _ ...SnapshotLoadBalancer` assertions and the `Use*` methods; the balancer now opts in solely through `SnapshotOptIn()`. The deprecated marker interfaces themselves are also deleted.
##########
pkg/cluster/loadbalancer/ringhash/ring_hash.go:
##########
@@ -58,6 +59,13 @@ func NewRingHash(config model.ConsistentHash, endpoints []*model.Endpoint) model
type RingHashing struct{}
+var _ loadbalancer.HealthyOnlySnapshotLoadBalancer = RingHashing{}
+var _ loadbalancer.ZeroCopySnapshotLoadBalancer = RingHashing{}
Review Comment:
Agreed — done in fceeb80d. Removed the `var _ ...SnapshotLoadBalancer` assertions and the `Use*` methods; the balancer now opts in solely through `SnapshotOptIn()`. The deprecated marker interfaces themselves are also deleted.
##########
pkg/cluster/loadbalancer/maglev/maglev_hash.go:
##########
@@ -50,6 +51,13 @@ func NewMaglevHash(config model.ConsistentHash, endpoints []*model.Endpoint) mod
type MaglevHash struct{}
+var _ loadbalancer.HealthyOnlySnapshotLoadBalancer = MaglevHash{}
+var _ loadbalancer.ZeroCopySnapshotLoadBalancer = MaglevHash{}
Review Comment:
Agreed — done in fceeb80d. Removed the `var _ ...SnapshotLoadBalancer` assertions and the `Use*` methods; the balancer now opts in solely through `SnapshotOptIn()`. The deprecated marker interfaces themselves are also deleted.
##########
pkg/cluster/loadbalancer/weightrandom/weight_random.go:
##########
@@ -40,6 +41,13 @@ type weightedEndpoint struct {
// It assigns weights to endpoints and uses these weights to influence the probability of selection.
type WeightRandom struct{}
+var _ loadbalancer.HealthyOnlySnapshotLoadBalancer = WeightRandom{}
+var _ loadbalancer.ZeroCopySnapshotLoadBalancer = WeightRandom{}
Review Comment:
Agreed — done in fceeb80d. Removed the `var _ ...SnapshotLoadBalancer` assertions and the `Use*` methods; the balancer now opts in solely through `SnapshotOptIn()`. The deprecated marker interfaces themselves are also deleted.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#issuecomment-4647084691
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=971) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go-pixiu&pullRequest=971&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=971&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go-pixiu&pullRequest=971&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go-pixiu&pullRequest=971)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] refactor(loadbalancer): gate snapshot fast paths behind an internal opt-in token [dubbo-go-pixiu]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #971:
URL: https://github.com/apache/dubbo-go-pixiu/pull/971#discussion_r3371168336
##########
pkg/cluster/loadbalancer/load_balancer.go:
##########
@@ -63,20 +64,45 @@ type SnapshotLoadBalancer interface {
HandlerWithSnapshot(c PickContext, policy model.LbPolicy) *model.Endpoint
}
-// HealthyOnlySnapshotLoadBalancer marks snapshot-aware balancers that do not
-// need PickContext.AllEndpoints. Unmarked snapshot balancers keep receiving
-// the full snapshot for compatibility with custom implementations.
+// HealthyOnlySnapshotLoadBalancer is retained for source compatibility with
+// callers that referenced the old marker contract.
+//
+// Deprecated: runtime fast-path decisions ignore this interface. Only trusted
+// in-tree balancers can opt in via SnapshotOptIn returning snapshotopt.Token.
type HealthyOnlySnapshotLoadBalancer interface {
UseHealthyEndpointsOnly() bool
}
-// ZeroCopySnapshotLoadBalancer marks trusted balancers that never mutate or
-// retain snapshot endpoints. Other snapshot balancers receive defensive
-// copies.
+// ZeroCopySnapshotLoadBalancer is retained for source compatibility with
+// callers that referenced the old marker contract.
+//
+// Deprecated: runtime fast-path decisions ignore this interface. Only trusted
+// in-tree balancers can opt in via SnapshotOptIn returning snapshotopt.Token.
type ZeroCopySnapshotLoadBalancer interface {
UseZeroCopySnapshot() bool
}
Review Comment:
PR description says the two exported marker interfaces were removed, but the code keeps `HealthyOnlySnapshotLoadBalancer` and `ZeroCopySnapshotLoadBalancer` (now marked Deprecated) for source compatibility. Please either update the PR description to match the implementation, or remove these exported interfaces if the intent is truly to delete them from the public surface.
##########
pkg/cluster/loadbalancer/ringhash/ring_hash.go:
##########
@@ -58,6 +59,13 @@ func NewRingHash(config model.ConsistentHash, endpoints []*model.Endpoint) model
type RingHashing struct{}
+var _ loadbalancer.HealthyOnlySnapshotLoadBalancer = RingHashing{}
+var _ loadbalancer.ZeroCopySnapshotLoadBalancer = RingHashing{}
Review Comment:
These compile-time assertions reference deprecated marker interfaces that no longer affect runtime behavior. Keeping them introduces new, unnecessary coupling to deprecated APIs. Prefer removing these `var _ ...SnapshotLoadBalancer` assertions and rely on `SnapshotOptIn()` for the fast-path opt-in.
##########
pkg/cluster/loadbalancer/rand/load_balancer_rand.go:
##########
@@ -32,6 +33,13 @@ func init() {
type Rand struct{}
+var _ loadbalancer.HealthyOnlySnapshotLoadBalancer = Rand{}
+var _ loadbalancer.ZeroCopySnapshotLoadBalancer = Rand{}
Review Comment:
These compile-time assertions reference deprecated marker interfaces that no longer affect runtime behavior. Keeping them introduces new, unnecessary coupling to deprecated APIs. Prefer removing these `var _ ...SnapshotLoadBalancer` assertions and rely on `SnapshotOptIn()` for the fast-path opt-in.
##########
pkg/cluster/loadbalancer/weightrandom/weight_random.go:
##########
@@ -40,6 +41,13 @@ type weightedEndpoint struct {
// It assigns weights to endpoints and uses these weights to influence the probability of selection.
type WeightRandom struct{}
+var _ loadbalancer.HealthyOnlySnapshotLoadBalancer = WeightRandom{}
+var _ loadbalancer.ZeroCopySnapshotLoadBalancer = WeightRandom{}
Review Comment:
These compile-time assertions reference deprecated marker interfaces that no longer affect runtime behavior. Keeping them introduces new, unnecessary coupling to deprecated APIs. Prefer removing these `var _ ...SnapshotLoadBalancer` assertions and rely on `SnapshotOptIn()` for the fast-path opt-in.
##########
pkg/cluster/loadbalancer/roundrobin/round_robin.go:
##########
@@ -32,6 +33,13 @@ func init() {
type RoundRobin struct{}
+var _ loadbalancer.HealthyOnlySnapshotLoadBalancer = RoundRobin{}
+var _ loadbalancer.ZeroCopySnapshotLoadBalancer = RoundRobin{}
Review Comment:
These compile-time assertions reference deprecated marker interfaces that no longer affect runtime behavior. Keeping them introduces new, unnecessary coupling to deprecated APIs. Prefer removing these `var _ ...SnapshotLoadBalancer` assertions and rely on `SnapshotOptIn()` for the fast-path opt-in.
##########
pkg/cluster/loadbalancer/load_balancer.go:
##########
@@ -63,20 +64,45 @@ type SnapshotLoadBalancer interface {
HandlerWithSnapshot(c PickContext, policy model.LbPolicy) *model.Endpoint
}
-// HealthyOnlySnapshotLoadBalancer marks snapshot-aware balancers that do not
-// need PickContext.AllEndpoints. Unmarked snapshot balancers keep receiving
-// the full snapshot for compatibility with custom implementations.
+// HealthyOnlySnapshotLoadBalancer is retained for source compatibility with
+// callers that referenced the old marker contract.
+//
+// Deprecated: runtime fast-path decisions ignore this interface. Only trusted
+// in-tree balancers can opt in via SnapshotOptIn returning snapshotopt.Token.
type HealthyOnlySnapshotLoadBalancer interface {
UseHealthyEndpointsOnly() bool
}
-// ZeroCopySnapshotLoadBalancer marks trusted balancers that never mutate or
-// retain snapshot endpoints. Other snapshot balancers receive defensive
-// copies.
+// ZeroCopySnapshotLoadBalancer is retained for source compatibility with
+// callers that referenced the old marker contract.
+//
+// Deprecated: runtime fast-path decisions ignore this interface. Only trusted
+// in-tree balancers can opt in via SnapshotOptIn returning snapshotopt.Token.
type ZeroCopySnapshotLoadBalancer interface {
UseZeroCopySnapshot() bool
}
+// snapshotOptInBalancer is the internal opt-in surface for trusted, in-tree
+// snapshot balancers. The method returns snapshotopt.Token, whose type lives in
+// an internal package, so only balancers under pkg/cluster/loadbalancer can
+// implement this interface. External plugins cannot name the return type and
+// therefore always fall through to the safe default (full snapshot, defensively
+// copied). This is the trust boundary described in issue #941.
+type snapshotOptInBalancer interface {
+ SnapshotOptIn() snapshotopt.Token
+}
Review Comment:
The new trust boundary (“external plugins cannot satisfy SnapshotOptIn because they can’t name snapshotopt.Token”) can be bypassed by *embedding* an in-tree balancer type that already has `SnapshotOptIn() snapshotopt.Token`. Method promotion would make the external type satisfy `snapshotOptInBalancer`, enabling the zero-copy / healthy-only fast paths for untrusted code.
Consider hardening `snapshotOptIn` with an additional runtime trust check (e.g., verify the concrete balancer’s `reflect.Type.PkgPath()` is under the in-tree `.../pkg/cluster/loadbalancer` prefix) before honoring the token, and update the comment to reflect the actual guarantee.
##########
pkg/cluster/loadbalancer/maglev/maglev_hash.go:
##########
@@ -50,6 +51,13 @@ func NewMaglevHash(config model.ConsistentHash, endpoints []*model.Endpoint) mod
type MaglevHash struct{}
+var _ loadbalancer.HealthyOnlySnapshotLoadBalancer = MaglevHash{}
+var _ loadbalancer.ZeroCopySnapshotLoadBalancer = MaglevHash{}
Review Comment:
These compile-time assertions reference deprecated marker interfaces that no longer affect runtime behavior. Keeping them introduces new, unnecessary coupling to deprecated APIs. Prefer removing these `var _ ...SnapshotLoadBalancer` assertions and rely on `SnapshotOptIn()` for the fast-path opt-in.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org