Skip to content

WIP: Remove tableId constructor for load balancers #5426

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ddanielr
Copy link
Contributor

Removes the single tableId constructor for SimpleLoadBalancer as it is no longer needed with the addition of the tablesToBalance map to the balancerParams.

This PR addresses the changes from #5363 but applied to main.

Will also need to remove the single table constructors that are marked as deprecated in #5425

Removes the single tableId constructor for SimpleLoadBalancer as it is
no longer needed with the addition of the tablesToBalance map to the
balancerParams.
@ddanielr ddanielr added this to the 4.0.0 milestone Mar 24, 2025
Comment on lines -69 to -77
// if tableToBalance is set, then only balance the given table
TableId tableToBalance = null;

public SimpleLoadBalancer() {}

public SimpleLoadBalancer(TableId table) {
tableToBalance = table;
}

Copy link
Member

Choose a reason for hiding this comment

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

Just adding context here: this constructor wasn't added to comply with the TabletBalancer SPI, but to make this implementation supportable by the delegating per-table balancer named TableLoadBalancer.

The whole reason TableLoadBalancer exists is to delegate to other balancers on a per-table basis. If that is no longer needed, because per-table delegation is now built-in at a higher level, then the whole TableLoadBalancer can be deleted... it is not needed anymore.

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