flow based tablet load balancer #16351

demmer · 2024-07-08T22:01:33Z

Description

Implement an optional flow-based tablet balancer.

The motivation and algorithm is described more in #12241 and in a long comment in the code.

In brief, the goal of this implementation is to more evenly balance out the queries to a the replicas in a shard when the tablets are not evenly distributed across the various cells where vtgates are receiving application traffic, because the default routing algorithm will always prefer an available tablet in the same cell as the vtgate, even if there are more idle tablets in other cells. The balancer addresses this by attempting to route an equal share to each tablet, preferring the same-cell where possible, but crossing cells in order to achieve better global balance.

Slack production results

Note that the original algorithm was developed many months ago, but only recently have we made the time to deploy this within Slack and spent some time to verify it works as expected.

The results from our initial testing are very encouraging and we have been running with this feature enabled for some time now on one of our main production keyspaces. This is a 16 shard keyspace with between 9-11 tablets per shard spread among our 4 active cells. Each cell receives the same amount of inbound traffic.

When we first enabled the feature shows that the wide disparity in QPS per replica goes away and instead the QPS is only based on the number of replicas per shard, as expected:

Since then we have actually normalized our deployment so each shard has 9 replicas and the QPS is quite even across all hosts, even though again they do not map evenly to each cell:

Related Issue(s)

#12241

Checklist

"Backport to:" labels have been added if this change should be back-ported to release branches
If this change is to be back-ported to previous releases, a justification is included in the PR description
Tests were added or are not required
Did the new or modified tests pass consistently locally and on CI?
Documentation was added or is not required Tabletbalancer docs website#1804

Deployment Notes

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com> Co-authored-by: Venkatraju V <venkatraju@slack-corp.com>

vitess-bot · 2024-07-08T22:01:35Z

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>

codecov · 2024-07-08T22:35:02Z

Codecov Report

Attention: Patch coverage is 83.22148% with 25 lines in your changes missing coverage. Please review.

Project coverage is 68.93%. Comparing base (feb845a) to head (707d77b).
Report is 34 commits behind head on main.

Files with missing lines	Patch %	Lines
go/vt/vtgate/tabletgateway.go	64.51%	11 Missing ⚠️
go/vt/vtgate/balancer/balancer.go	92.03%	9 Missing ⚠️
go/vt/vtgate/vtgate.go	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16351      +/-   ##
==========================================
- Coverage   68.98%   68.93%   -0.05%     
==========================================
  Files        1562     1565       +3     
  Lines      200697   201473     +776     
==========================================
+ Hits       138446   138886     +440     
- Misses      62251    62587     +336

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

systay · 2024-07-09T09:08:46Z

This looks great @demmer

Just need to use rand/v2 to make CI happy, and then we need a website PR for the new flags, right?

Signed-off-by: Venkatraju V <venkatraju@slack-corp.com>

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>

demmer · 2024-07-10T20:54:29Z

@systay based on the test coverage reports, @venkatraju and I added some more tests for the balancer, and I beefed up some of the existing tests for the retry logic, including running both with and without the balancer. might be overkill but this is kinda a critical part of the code :)

go/vt/vtgate/tabletgateway.go

deepthi · 2024-07-10T23:46:56Z

go/vt/vtgate/tabletgateway.go

@@ -62,6 +70,9 @@ func init() {
 		fs.StringVar(&CellsToWatch, "cells_to_watch", "", "comma-separated list of cells for watching tablets")
 		fs.DurationVar(&initialTabletTimeout, "gateway_initial_tablet_timeout", 30*time.Second, "At startup, the tabletGateway will wait up to this duration to get at least one tablet per keyspace/shard/tablet type")
 		fs.IntVar(&retryCount, "retry-count", 2, "retry count")
+		fs.BoolVar(&balancerEnabled, "balancer_enabled", false, "Whether to enable the tablet balancer to evenly spread query load")
+		fs.StringSliceVar(&balancerVtgateCells, "balancer_vtgate_cells", []string{}, "When in balanced mode, a comma-separated list of cells that contain vtgates (required)")


Suggested change

fs.StringSliceVar(&balancerVtgateCells, "balancer_vtgate_cells", []string{}, "When in balanced mode, a comma-separated list of cells that contain vtgates (required)")

fs.StringSliceVar(&balancerVtgateCells, "balancer-vtgate-cells", []string{}, "When in balanced mode, a comma-separated list of cells that contain vtgates (required)")

Everything else is cosmetic, but this seems important.
What happens if you add vtgates to a cell that didn't previously have any? You would have to restart every vtgate with a new value for this flag?
It may not be very common in practice, but it should be documented somewhere.
Of course, the real answer is that vtgates should be given a list of all cells and discover which ones contain vtgates, but we've so far avoided adding vtgates to the topo.

What happens if you add vtgates to a cell that didn't previously have any? You would have to restart every vtgate with a new value for this flag?

Yep. For the current algorithm to work as designed, it's required that all the vtgates know the fully set of cells that are running vtgates and that the load among these cells is roughly even. This allows each individual vtgate to make the assumption about what the others will do without needing signaling.

If you want to change this assumption therefore, you do have to restart the whole fleet to take it into account, just like any other vtgate flag change.

It may not be very common in practice, but it should be documented somewhere.

That makes sense. I'll include something to this effect when we write the website docs.

Of course, the real answer is that vtgates should be given a list of all cells and discover which ones contain vtgates, but we've so far avoided adding vtgates to the topo.

Yeah - that's obviously beyond the scope of this PR, and it would in theory reduce the dependency that query inflows per cell need to be balanced (i.e. you could use the number of gates as a proxy for how much load you expect).

deepthi · 2024-07-10T23:47:06Z

go/vt/vtgate/tabletgateway.go

@@ -62,6 +70,9 @@ func init() {
 		fs.StringVar(&CellsToWatch, "cells_to_watch", "", "comma-separated list of cells for watching tablets")
 		fs.DurationVar(&initialTabletTimeout, "gateway_initial_tablet_timeout", 30*time.Second, "At startup, the tabletGateway will wait up to this duration to get at least one tablet per keyspace/shard/tablet type")
 		fs.IntVar(&retryCount, "retry-count", 2, "retry count")
+		fs.BoolVar(&balancerEnabled, "balancer_enabled", false, "Whether to enable the tablet balancer to evenly spread query load")
+		fs.StringSliceVar(&balancerVtgateCells, "balancer_vtgate_cells", []string{}, "When in balanced mode, a comma-separated list of cells that contain vtgates (required)")
+		fs.StringSliceVar(&balancerKeyspaces, "balancer_keyspaces", []string{}, "When in balanced mode, a comma-separated list of keyspaces for which to use the balancer (optional)")


Suggested change

fs.StringSliceVar(&balancerKeyspaces, "balancer_keyspaces", []string{}, "When in balanced mode, a comma-separated list of keyspaces for which to use the balancer (optional)")

fs.StringSliceVar(&balancerKeyspaces, "balancer-keyspaces", []string{}, "When in balanced mode, a comma-separated list of keyspaces for which to use the balancer (optional)")

I'm curious, why was this added? So that you can opt in and out per keyspace as a phased rollout?

Yep exactly. That's how we did the rollout testing inside Slack -- started with a loadtest keyspace only and then moved to the one production keyspace where we saw the most imbalance, so we could focus on that without worrying about potential negative impact on the rest of the (thousands of) shards.

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>

deepthi · 2024-08-27T17:36:16Z

@venkatraju looks like this may need either a merge from main, or a simpler code change to pass linter checks.

Signed-off-by: Venkatraju V <venkatraju@slack-corp.com>

venkatraju · 2024-08-27T17:54:38Z

@venkatraju looks like this may need either a merge from main, or a simpler code change to pass linter checks.

@deepthi Fixed the log line that was triggering the linter error.

deepthi · 2024-08-28T17:18:25Z

@systay @rohit-nayak-ps do either of you think this needs a release note?

I've added the label to block merging. This will be a nice thing to add to the release notes.
@venkatraju can you please add a section to the summary release notes doc for this feature? It should go into https://github.com/vitessio/vitess/blob/main/changelog/21.0/21.0.0/summary.md

Signed-off-by: Venkatraju V <venkatraju@slack-corp.com>

Signed-off-by: Venkatraju <venkatraju@gmail.com>

venkatraju · 2024-08-29T00:25:01Z

@venkatraju can you please add a section to the summary release notes doc for this feature? It should go into https://github.com/vitessio/vitess/blob/main/changelog/21.0/21.0.0/summary.md

@deepthi added release notes. Could you review the text, and also comment on whether you want the new flags called out there?

(sorry about the messed up git history from a rebase that went wrong)

deepthi · 2024-08-29T15:47:19Z

@venkatraju can you please add a section to the summary release notes doc for this feature? It should go into https://github.com/vitessio/vitess/blob/main/changelog/21.0/21.0.0/summary.md

@deepthi added release notes. Could you review the text, and also comment on whether you want the new flags called out there?

(sorry about the messed up git history from a rebase that went wrong)

This looks fine. Details on flags will be in the vtgate command reference (auto generated).

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com> Signed-off-by: Venkatraju V <venkatraju@slack-corp.com> Signed-off-by: Venkatraju <venkatraju@gmail.com> Co-authored-by: Venkatraju V <venkatraju@slack-corp.com> Co-authored-by: Venkatraju <venkatraju@gmail.com>

* flow based tablet load balancer (vitessio#16351) Signed-off-by: Michael Demmer <mdemmer@slack-corp.com> Signed-off-by: Venkatraju V <venkatraju@slack-corp.com>

demmer and others added 2 commits July 8, 2024 14:40

implement TabletBalancer

73b16ba

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com> Co-authored-by: Venkatraju V <venkatraju@slack-corp.com>

integrate tabletbalancer into vtgate

4fd193b

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com> Co-authored-by: Venkatraju V <venkatraju@slack-corp.com>

demmer requested review from harshit-gangal, systay, frouioui, GuptaManan100 and deepthi as code owners July 8, 2024 22:01

github-actions bot added this to the v21.0.0 milestone Jul 8, 2024

oops fix typo

6f47d9d

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>

venkatraju and others added 2 commits July 9, 2024 09:13

tabletbalancer test coverage for tablet replaced

e10b474

Signed-off-by: Venkatraju V <venkatraju@slack-corp.com>

add tests for retry

f17861b

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>

deepthi reviewed Jul 11, 2024

View reviewed changes

demmer added 4 commits July 11, 2024 07:15

use math/rand/v2

b85fc90

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>

add test helper to check the right count

d63f510

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>

use - instead of _ in argumnet names

1194a5e

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>

update flags test expectation

b9bd190

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>

demmer mentioned this pull request Aug 1, 2024

Tabletbalancer docs vitessio/website#1804

Merged

remove redundant log in test

b3fbaaa

Signed-off-by: Venkatraju V <venkatraju@slack-corp.com>

venkatraju force-pushed the tabletbalancer-v21 branch from ac6b050 to b3fbaaa Compare August 27, 2024 17:48

venkatraju mentioned this pull request Aug 27, 2024

balancer flags renamed to match upstream slackhq/vitess#496

Merged

4 tasks

deepthi added the release notes (needs details) This PR needs to be listed in the release notes in a dedicated section (deprecation notice, etc...) label Aug 28, 2024

venkatraju force-pushed the tabletbalancer-v21 branch 2 times, most recently from b2f8253 to 3b889d6 Compare August 29, 2024 00:04

typo in flag name

66595ac

Signed-off-by: Venkatraju V <venkatraju@slack-corp.com>

venkatraju force-pushed the tabletbalancer-v21 branch from 3b889d6 to 9aafb2a Compare August 29, 2024 00:09

venkatraju requested review from mattlord, ajm188, notfelineit, rohit-nayak-ps, shlomi-noach, derekperkins, timvaillancourt, arthurschreiber and vmg as code owners August 29, 2024 00:09

add release notes

5ae7720

Signed-off-by: Venkatraju V <venkatraju@slack-corp.com>

venkatraju force-pushed the tabletbalancer-v21 branch from 9aafb2a to 5ae7720 Compare August 29, 2024 00:16

Merge branch 'main' into tabletbalancer-v21

707d77b

Signed-off-by: Venkatraju <venkatraju@gmail.com>

systay removed the release notes (needs details) This PR needs to be listed in the release notes in a dedicated section (deprecation notice, etc...) label Aug 29, 2024

deepthi merged commit e799299 into vitessio:main Aug 29, 2024
129 of 130 checks passed

venkatraju mentioned this pull request Aug 29, 2024

flow based tablet load balancer (#16351) slackhq/vitess#498

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

flow based tablet load balancer #16351

flow based tablet load balancer #16351

demmer commented Jul 8, 2024 •

edited by deepthi

Loading

vitess-bot bot commented Jul 8, 2024 •

edited by deepthi

Loading

codecov bot commented Jul 8, 2024 •

edited

Loading

systay commented Jul 9, 2024

demmer commented Jul 10, 2024

deepthi Jul 10, 2024

deepthi Jul 11, 2024

demmer Jul 11, 2024

demmer Jul 11, 2024

deepthi Jul 10, 2024

deepthi Jul 10, 2024

demmer Jul 11, 2024

deepthi commented Aug 27, 2024

venkatraju commented Aug 27, 2024

deepthi commented Aug 28, 2024

venkatraju commented Aug 29, 2024

deepthi commented Aug 29, 2024

	fs.StringSliceVar(&balancerVtgateCells, "balancer_vtgate_cells", []string{}, "When in balanced mode, a comma-separated list of cells that contain vtgates (required)")
	fs.StringSliceVar(&balancerVtgateCells, "balancer-vtgate-cells", []string{}, "When in balanced mode, a comma-separated list of cells that contain vtgates (required)")

	fs.StringSliceVar(&balancerKeyspaces, "balancer_keyspaces", []string{}, "When in balanced mode, a comma-separated list of keyspaces for which to use the balancer (optional)")
	fs.StringSliceVar(&balancerKeyspaces, "balancer-keyspaces", []string{}, "When in balanced mode, a comma-separated list of keyspaces for which to use the balancer (optional)")

flow based tablet load balancer #16351

flow based tablet load balancer #16351

Conversation

demmer commented Jul 8, 2024 • edited by deepthi Loading

Description

Slack production results

Related Issue(s)

Checklist

Deployment Notes

vitess-bot bot commented Jul 8, 2024 • edited by deepthi Loading

Review Checklist

General

Tests

Documentation

New flags

If a workflow is added or modified:

Backward compatibility

codecov bot commented Jul 8, 2024 • edited Loading

Codecov Report

systay commented Jul 9, 2024

demmer commented Jul 10, 2024

deepthi Jul 10, 2024

Choose a reason for hiding this comment

deepthi Jul 11, 2024

Choose a reason for hiding this comment

demmer Jul 11, 2024

Choose a reason for hiding this comment

demmer Jul 11, 2024

Choose a reason for hiding this comment

deepthi Jul 10, 2024

Choose a reason for hiding this comment

deepthi Jul 10, 2024

Choose a reason for hiding this comment

demmer Jul 11, 2024

Choose a reason for hiding this comment

deepthi commented Aug 27, 2024

venkatraju commented Aug 27, 2024

deepthi commented Aug 28, 2024

venkatraju commented Aug 29, 2024

deepthi commented Aug 29, 2024

demmer commented Jul 8, 2024 •

edited by deepthi

Loading

vitess-bot bot commented Jul 8, 2024 •

edited by deepthi

Loading

codecov bot commented Jul 8, 2024 •

edited

Loading