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

examples: Add custom load balancer example #6691

Merged
merged 20 commits into from
Apr 17, 2024

Conversation

zasweq
Copy link
Contributor

@zasweq zasweq commented Oct 5, 2023

Contains #6679. This PR adds a custom load balancer example, deployed as the top level balancer of the channel through a service config. This custom load balancer is a petiole policy wrapping pick first, which is the intended way we want users to write their own custom load balancers, as users get features such as Sticky Transient Failure, Health Checking, and the Happy Eyeballs algorithm out of the box.

As part of this PR, pick_first is changed to work on the endpoints list and handle the endpoints list as defined in A61. This change works within system because the Channel is already emitting a Slice of Endpoints.

A helper interface between the petiole policy and the base pick first is also added. This lets petiole policy focus on the business logic, as seen by the simplicitly of custom_round_robin.

RELEASE NOTES:

  • examples: Add custom lb example

@zasweq zasweq added the Type: Documentation Documentation or examples label Oct 5, 2023
@zasweq zasweq added this to the 1.59 Release milestone Oct 5, 2023
@zasweq zasweq requested a review from dfawley October 5, 2023 18:01
@ginayeh ginayeh modified the milestones: 1.59 Release, 1.60 Release Oct 5, 2023
@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Merging #6691 (944499d) into master (adf976b) will decrease coverage by 0.47%.
Report is 6 commits behind head on master.
The diff coverage is 13.21%.

❗ Current head 944499d differs from pull request most recent head 7af9e93. Consider uploading reports for the commit 7af9e93 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6691      +/-   ##
==========================================
- Coverage   81.24%   80.77%   -0.47%     
==========================================
  Files         345      346       +1     
  Lines       33941    34094     +153     
==========================================
- Hits        27574    27541      -33     
- Misses       5202     5379     +177     
- Partials     1165     1174       +9     
Files Coverage Δ
pickfirst.go 82.69% <88.46%> (+0.47%) ⬆️
balancer/balanceraggregator/balanceraggregator.go 0.00% <0.00%> (ø)

... and 24 files with indirect coverage changes

@zasweq zasweq force-pushed the add-custom-lb-example branch from fbd54b9 to 9521bb6 Compare October 16, 2023 20:23
@ginayeh
Copy link
Contributor

ginayeh commented Oct 31, 2023

Please rebase your comment before merging.

@zasweq
Copy link
Contributor Author

zasweq commented Oct 31, 2023

Done.

@arvindbr8 arvindbr8 modified the milestones: 1.60 Release, 1.61 Release Nov 14, 2023
@zasweq zasweq modified the milestones: 1.61 Release, 1.62 Release Jan 23, 2024
@ginayeh ginayeh modified the milestones: 1.62 Release, 1.63 Release Feb 8, 2024
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

I have a lot of comments. Maybe we should just simplify this for now and make something small that doesn't need to worry about being a petiole policy just so we can get it done with a reasonable amount of effort.

examples/features/customloadbalancer/README.md Outdated Show resolved Hide resolved
pickfirst.go Outdated Show resolved Hide resolved
pickfirst.go Outdated Show resolved Hide resolved
pickfirst.go Outdated Show resolved Hide resolved
examples/features/customloadbalancer/README.md Outdated Show resolved Hide resolved
examples/features/customloadbalancer/README.md Outdated Show resolved Hide resolved
@dfawley dfawley assigned zasweq and unassigned dfawley Mar 6, 2024
@zasweq zasweq modified the milestones: 1.63 Release, 1.64 Release Mar 20, 2024
Copy link
Contributor Author

@zasweq zasweq left a comment

Choose a reason for hiding this comment

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

Got to all comments. Thanks for the pass.

examples/features/customloadbalancer/README.md Outdated Show resolved Hide resolved
pickfirst.go Outdated Show resolved Hide resolved
pickfirst.go Outdated Show resolved Hide resolved
examples/features/customloadbalancer/README.md Outdated Show resolved Hide resolved
examples/features/customloadbalancer/README.md Outdated Show resolved Hide resolved
@zasweq zasweq force-pushed the add-custom-lb-example branch from 7aafeef to 0994c23 Compare March 28, 2024 22:46
@zasweq
Copy link
Contributor Author

zasweq commented Mar 28, 2024

Added BalancerAggregator utility we discussed offline :). This is ready to review now.

@zasweq zasweq assigned dfawley and unassigned zasweq Mar 28, 2024
@dfawley dfawley assigned zasweq and unassigned dfawley Mar 29, 2024
@zasweq zasweq force-pushed the add-custom-lb-example branch from 8780a34 to 3e18a93 Compare April 12, 2024 22:41
@zasweq zasweq force-pushed the add-custom-lb-example branch from 7e0b07d to aead17f Compare April 13, 2024 00:52
@dfawley dfawley assigned zasweq and unassigned dfawley Apr 15, 2024
@zasweq zasweq assigned dfawley and unassigned zasweq Apr 15, 2024
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Can you please add a simple test or 2 into the endpointsharding (or whatever we call it) package, please?

@dfawley dfawley assigned zasweq and unassigned dfawley Apr 15, 2024
@zasweq
Copy link
Contributor Author

zasweq commented Apr 17, 2024

Added e2e test that tested the main behaviors I view endpoint sharding to have (a child per endpoint, default behavior of round robin over pickers in aggregated state, access to child picker in parent UpdateState).

@aranjans aranjans assigned dfawley and unassigned zasweq Apr 17, 2024
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

🎉

Now to rewrite round_robin 😆

@dfawley dfawley assigned zasweq and unassigned dfawley Apr 17, 2024
@zasweq
Copy link
Contributor Author

zasweq commented Apr 17, 2024

And move pick first :).

@zasweq zasweq merged commit 431436d into grpc:master Apr 17, 2024
12 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Documentation Documentation or examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants