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 weighted load balancer #6315

Merged
merged 15 commits into from
Oct 5, 2024
Merged

Conversation

Shaddoll
Copy link
Member

@Shaddoll Shaddoll commented Sep 28, 2024

What changed?
Introduce a weighted load balancer to matching client

Why?
See the documentation in the PR.
This loadbalancer is better than the previous 2 implementations.

How did you test it?
unit test, and also verified in dev environment

Potential risks

Release notes

Documentation Changes

Copy link

codecov bot commented Oct 2, 2024

Codecov Report

Attention: Patch coverage is 90.11628% with 17 lines in your changes missing coverage. Please review.

Project coverage is 73.60%. Comparing base (225c0ea) to head (7c2a13b).
Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
client/matching/weighted_loadbalancer.go 90.90% 5 Missing and 5 partials ⚠️
service/matching/handler/engine.go 0.00% 3 Missing ⚠️
service/matching/tasklist/task_list_manager.go 60.00% 1 Missing and 1 partial ⚠️
client/matching/loadbalancer.go 83.33% 1 Missing ⚠️
client/matching/rr_loadbalancer.go 83.33% 1 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
client/matching/client.go 90.55% <100.00%> (+1.07%) ⬆️
client/matching/multi_loadbalancer.go 100.00% <100.00%> (ø)
client/matching/loadbalancer.go 69.69% <83.33%> (-0.31%) ⬇️
client/matching/rr_loadbalancer.go 94.91% <83.33%> (ø)
service/matching/tasklist/task_list_manager.go 68.53% <60.00%> (-0.18%) ⬇️
service/matching/handler/engine.go 79.69% <0.00%> (-0.21%) ⬇️
client/matching/weighted_loadbalancer.go 90.90% <90.90%> (ø)

... and 42 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 225c0ea...7c2a13b. Read the comment docs.

@Shaddoll Shaddoll changed the title wip: Introduce weighted load balancer Introduce weighted load balancer Oct 3, 2024
client/matching/multi_loadbalancer.go Show resolved Hide resolved
client/matching/multi_loadbalancer.go Show resolved Hide resolved
client/matching/weighted_loadbalancer.go Show resolved Hide resolved
pw.initialized = true
}

func NewWeightedLoadBalancer(
Copy link
Member

Choose a reason for hiding this comment

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

I recommend adding a background ticker to log weights. (need to add start/stop to load balancer interface for this.)
This will help a lot for debugging in prod. Global rate limiter is missing this and we are having hard time debugging edge cases.

client/matching/weighted_loadbalancer.go Show resolved Hide resolved
@Shaddoll Shaddoll merged commit 5b92d52 into cadence-workflow:master Oct 5, 2024
22 checks passed
@Shaddoll Shaddoll deleted the weight branch October 5, 2024 00:43
Shaddoll added a commit to Shaddoll/cadence that referenced this pull request Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants