-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
vtorc
: fetch all tablets from cells once + filter during refresh
#17388
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
vtorc
: fetch all tablets from cells once and filtervtorc
: fetch all tablets from cells once + filter during refresh
Signed-off-by: Tim Vaillancourt <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17388 +/- ##
==========================================
+ Coverage 67.47% 67.51% +0.03%
==========================================
Files 1581 1581
Lines 253944 253972 +28
==========================================
+ Hits 171353 171465 +112
+ Misses 82591 82507 -84 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
This introduces a problem if shards are added/removed. Moving back to draft |
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Signed-off-by: Tim Vaillancourt <[email protected]>
Description
This PR changes VTOrc tablet discovery to fetch all tablets once and filter them client side
Today, when
--clusters_to_watch
is set, VTOrc will callrefreshTabletsInKeyspaceShard(...)
in a goroutine for every shard when refreshing tablets at the interval--topo-information-refresh-duration
Unfortunately calling
refreshTabletsInKeyspaceShard(...)
is very inefficient because behind the scenes it is just loading all tablets from all cells because fetching just 1-shard or keyspace from the topo store isn't possible without fetching "all" 😱This means that today a single refresh of all tablets causes
shards x # tablets
to be read instead of just# tablets
, and much of the data read is just duplicated. Instead this PR causes only# of tablets
to be fetched each tickRelated Issue(s)
Tracking issue: #17330
Checklist
Deployment Notes