chenBright commented on code in PR #3426:
URL: https://github.com/apache/brpc/pull/3426#discussion_r3733250042
##########
src/brpc/policy/weighted_randomized_load_balancer.cpp:
##########
@@ -131,7 +131,11 @@ int WeightedRandomizedLoadBalancer::SelectServer(const
SelectIn& in, SelectOut*
uint64_t weight_sum = s->weight_sum;
for (size_t i = 0; i < n; ++i) {
uint64_t random_weight = butil::fast_rand_less_than(weight_sum);
- const Server random_server(0, 0, random_weight);
+ // current_weight_sum is an inclusive prefix sum, so server i owns the
+ // half-open range [prefix(i-1), prefix(i)). random_weight belongs to
the
+ // first server whose prefix sum is strictly greater than it, which is
+ // lower_bound() of random_weight + 1 rather than of random_weight
itself.
+ const Server random_server(0, 0, random_weight + 1);
const auto& server =
std::lower_bound(s->server_list.begin(), s->server_list.end(),
random_server, server_compare);
Review Comment:
I'd suggest expressing the intent directly with `upper_bound` instead. The
whole
point is "find the first prefix sum strictly greater than `random_weight`",
and that is
precisely what `upper_bound` means. The `+ 1` form encodes the same thing
indirectly, which
is why it needs four lines of comment to explain, and it also makes every
reader stop and
re-check whether the increment can overflow.
--
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]
