Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce WeightedRandomDistributionSelector (xDS-endpoint pt 1) #5501

Merged
merged 2 commits into from
Mar 22, 2024

Conversation

jrhee17
Copy link
Contributor

@jrhee17 jrhee17 commented Mar 13, 2024

Motivation:

EdfLoadBalancer is a basic load balancer which distributes entries based on weight using Earliest Deadline First Scheduling.

ref: https://github.com/envoyproxy/envoy/blob/b7818b0df716af47ec22982c5a1cbbace5f2ae15/source/common/upstream/load_balancer_impl.h#L508

This is used not only for request load balancing, but also load balancing based on Locality

ref: https://github.com/envoyproxy/envoy/blob/b7818b0df716af47ec22982c5a1cbbace5f2ae15/source/common/upstream/upstream_impl.cc#L625

Within our codebase, it seems like we could use WeightedRandomDistributionEndpointSelector easily for this purpose. I propose that we extract a WeightedRandomDistributionSelector and make it publicly available.

See the following for a sample use-case in #5450
ref:

private final WeightedRandomDistributionSelector<LocalityEntry> healthyLocalitySelector;
private final WeightedRandomDistributionSelector<LocalityEntry> degradedLocalitySelector;

POC: #5450

Modifications:

  • Extract WeightedRandomDistributionSelector and move it to the internal package.

Result:

  • WeightedRandomDistributionSelector is now ready to be reused.

@github-actions github-actions bot requested review from ikhoon, minwoox and trustin March 13, 2024 15:26
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 92.59259% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 74.04%. Comparing base (4bfa172) to head (05a4594).

❗ Current head 05a4594 differs from pull request most recent head 8892a44. Consider uploading reports for the commit 8892a44 to get more accurate results

Files Patch % Lines
...t/endpoint/WeightedRandomDistributionSelector.java 91.83% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5501       +/-   ##
===========================================
+ Coverage        0   74.04%   +74.04%     
- Complexity      0    20801    +20801     
===========================================
  Files           0     1802     +1802     
  Lines           0    76578    +76578     
  Branches        0     9760     +9760     
===========================================
+ Hits            0    56699    +56699     
- Misses          0    15259    +15259     
- Partials        0     4620     +4620     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jrhee17 jrhee17 added this to the 1.28.0 milestone Mar 14, 2024
@jrhee17 jrhee17 marked this pull request as ready for review March 14, 2024 08:48
Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I didn't realize that we could use WeightedRandomDistributionSelector instead of the EDF scheduler which uses deadline with the priority queue. 😉

@minwoox minwoox merged commit c6b8906 into line:main Mar 22, 2024
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants