You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "RainYuY (via GitHub)" <gi...@apache.org> on 2025/12/15 15:38:48 UTC
[PR] feat(loadbalance): optimize invoker selection logic for improved performance [dubbo-go]
RainYuY opened a new pull request, #3114:
URL: https://github.com/apache/dubbo-go/pull/3114
(no comment)
--
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] feat(loadbalance): optimize invoker selection logic for improved performance [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on PR #3114:
URL: https://github.com/apache/dubbo-go/pull/3114#issuecomment-3658542250
1. the base branch should be develop, the modification for `tools/dubbogo-cli/generator/sample/hessian/constant.go` and `tools/dubbogo-cli/generator/sample/hessian/generator.go` are the commit in main branch, do not commit them to develop branch
2. the modification for `Makefile` has already been done in https://github.com/apache/dubbo-go/pull/3109 u don't need to modify `Makefile`, the CI problem have been fixed in develop branch, u just need to make sure the target branch is develop.
--
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] feat(loadbalance): optimize invoker selection logic for improved performance [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3114:
URL: https://github.com/apache/dubbo-go/pull/3114#issuecomment-3656352455
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3114) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3114&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3114&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3114&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3114&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3114&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3114)
--
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] feat(loadbalance): optimize invoker selection logic for improved performance [dubbo-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3114:
URL: https://github.com/apache/dubbo-go/pull/3114#issuecomment-3658349895
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3114?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 `45.45455%` with `6 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 40.34%. Comparing base ([`c63bec0`](https://app.codecov.io/gh/apache/dubbo-go/commit/c63bec05cea73237e0ece3ac41cc3b75143b26ba?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2e51505`](https://app.codecov.io/gh/apache/dubbo-go/commit/2e51505fe587663e70495572a12affd2ee19b510?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/dubbo-go/pull/3114?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [cluster/loadbalance/random/loadbalance.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3114?src=pr&el=tree&filepath=cluster%2Floadbalance%2Frandom%2Floadbalance.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y2x1c3Rlci9sb2FkYmFsYW5jZS9yYW5kb20vbG9hZGJhbGFuY2UuZ28=) | 45.45% | [4 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3114?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 @@
## main #3114 +/- ##
==========================================
- Coverage 40.35% 40.34% -0.01%
==========================================
Files 457 457
Lines 32415 32423 +8
==========================================
+ Hits 13080 13082 +2
- Misses 18073 18077 +4
- Partials 1262 1264 +2
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo-go/pull/3114?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.
</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] feat(loadbalance): optimize invoker selection logic for improved performance [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3114:
URL: https://github.com/apache/dubbo-go/pull/3114#issuecomment-3658337433
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3114) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3114&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3114&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3114&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3114&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3114&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3114)
--
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] feat(loadbalance): optimize invoker selection logic for improved performance [dubbo-go]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3114:
URL: https://github.com/apache/dubbo-go/pull/3114#discussion_r2621578265
##########
cluster/loadbalance/random/loadbalance.go:
##########
@@ -70,11 +71,21 @@ func (lb *randomLoadBalance) Select(invokers []base.Invoker, invocation base.Inv
if totalWeight > 0 && !sameWeight {
// If (not every invoker has the same weight & at least one invoker's weight>0), select randomly based on totalWeight.
offset := rand.Int63n(totalWeight)
-
- for i := 0; i < length; i++ {
- if offset < weights[i] {
- return invokers[i]
+ if length <= 4 {
+ for i := 0; i < length; i++ {
+ if offset < weights[i] {
+ return invokers[i]
+ }
+ }
+ } else {
+ idx := sort.Search(len(weights), func(i int) bool { return weights[i] >= offset })
+ if idx < length && weights[idx] == offset {
+ for idx+1 < length && weights[idx+1] == offset {
+ idx++
+ }
+ idx++
}
Review Comment:
The binary search implementation has a critical bug. The condition `weights[idx] >= offset` will find the first index where the cumulative weight is greater than or equal to offset, but the special handling when `weights[idx] == offset` is incorrect.
The original loop returns the invoker when `offset < weights[i]`, which correctly handles the case where offset falls within the weight range of invoker i. However, the binary search logic attempts to handle the equality case by incrementing idx, which breaks the weighted random selection algorithm.
For example, if offset=5 and weights=[3,5,10], the original code would return invokers[2] (since 5 < 10 is the first true condition), but the new code would find idx=1 (weights[1]==5), then increment to idx=2, then return invokers[2]. While this might work in some cases, when offset equals a cumulative weight boundary, the algorithm should return the invoker at that boundary index, not skip past it.
The correct binary search should use `weights[i] > offset` as the search predicate to match the original `offset < weights[i]` logic.
```suggestion
idx := sort.Search(len(weights), func(i int) bool { return weights[i] > offset })
```
##########
cluster/loadbalance/random/loadbalance.go:
##########
@@ -70,11 +71,21 @@ func (lb *randomLoadBalance) Select(invokers []base.Invoker, invocation base.Inv
if totalWeight > 0 && !sameWeight {
// If (not every invoker has the same weight & at least one invoker's weight>0), select randomly based on totalWeight.
offset := rand.Int63n(totalWeight)
-
- for i := 0; i < length; i++ {
- if offset < weights[i] {
- return invokers[i]
+ if length <= 4 {
+ for i := 0; i < length; i++ {
+ if offset < weights[i] {
+ return invokers[i]
+ }
+ }
+ } else {
+ idx := sort.Search(len(weights), func(i int) bool { return weights[i] >= offset })
+ if idx < length && weights[idx] == offset {
+ for idx+1 < length && weights[idx+1] == offset {
+ idx++
+ }
+ idx++
}
+ return invokers[idx]
Review Comment:
The binary search implementation is missing bounds checking. When `sort.Search` doesn't find a matching element, it returns `len(weights)`. If this happens, accessing `invokers[idx]` at line 88 will cause an index out of bounds panic.
While the condition at line 82 checks `idx < length`, this check only applies to the special equality case. If the equality condition is false, the code proceeds to line 88 without verifying that `idx < length`, which could cause a runtime panic.
```suggestion
if idx < length {
return invokers[idx]
}
// Defensive fallback: should not happen, but return a random invoker to avoid panic
return invokers[rand.Intn(length)]
```
##########
Makefile:
##########
@@ -42,7 +42,7 @@ test: clean
fmt: install-imports-formatter
# replace interface{} with any
- go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -category=efaceany -fix -test ./...
+ go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -any=true -fix -test ./...
Review Comment:
The Makefile change appears unrelated to the PR's stated purpose of optimizing invoker selection logic. The change from `-category=efaceany` to `-any=true` is a modification to the code formatting command, which should be in a separate commit or PR focused on build/tooling updates rather than being bundled with a performance optimization change.
```suggestion
go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -category=efaceany -fix -test ./...
```
##########
cluster/loadbalance/random/loadbalance.go:
##########
@@ -70,11 +71,21 @@ func (lb *randomLoadBalance) Select(invokers []base.Invoker, invocation base.Inv
if totalWeight > 0 && !sameWeight {
// If (not every invoker has the same weight & at least one invoker's weight>0), select randomly based on totalWeight.
offset := rand.Int63n(totalWeight)
-
- for i := 0; i < length; i++ {
- if offset < weights[i] {
- return invokers[i]
+ if length <= 4 {
Review Comment:
The threshold value of 4 for choosing between linear search and binary search appears arbitrary and lacks justification. Binary search has O(log n) complexity but includes overhead that may not be beneficial for small arrays.
While optimizing for larger arrays makes sense, the choice of 4 as the cutoff should be based on benchmarking data. Typically, for such optimizations, the threshold might be higher (e.g., 10-20 elements) depending on the specific use case and performance characteristics. Consider providing justification for this value or making it configurable.
##########
cluster/loadbalance/random/loadbalance.go:
##########
@@ -70,11 +71,21 @@ func (lb *randomLoadBalance) Select(invokers []base.Invoker, invocation base.Inv
if totalWeight > 0 && !sameWeight {
// If (not every invoker has the same weight & at least one invoker's weight>0), select randomly based on totalWeight.
offset := rand.Int63n(totalWeight)
-
- for i := 0; i < length; i++ {
- if offset < weights[i] {
- return invokers[i]
+ if length <= 4 {
+ for i := 0; i < length; i++ {
+ if offset < weights[i] {
+ return invokers[i]
+ }
+ }
+ } else {
+ idx := sort.Search(len(weights), func(i int) bool { return weights[i] >= offset })
+ if idx < length && weights[idx] == offset {
+ for idx+1 < length && weights[idx+1] == offset {
+ idx++
+ }
+ idx++
}
+ return invokers[idx]
}
Review Comment:
The new binary search optimization path (for length > 4) lacks test coverage. The existing tests only verify basic selection and weighted selection but don't specifically validate the correctness of the binary search implementation with different array sizes and weight distributions.
Consider adding test cases that:
1. Test invoker selection with exactly 5 invokers to trigger the binary search path
2. Verify correct behavior when offset equals cumulative weight boundaries
3. Test edge cases like all weights being equal in the binary search path
--
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] feat(loadbalance): optimize invoker selection logic for improved performance [dubbo-go]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY closed pull request #3114: feat(loadbalance): optimize invoker selection logic for improved performance
URL: https://github.com/apache/dubbo-go/pull/3114
--
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] feat(loadbalance): optimize invoker selection logic for improved performance [dubbo-go]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on PR #3114:
URL: https://github.com/apache/dubbo-go/pull/3114#issuecomment-3658629645
Rebase onto the develop branch, then resubmit.
--
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