-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Throttler: Store Config in Global Keyspace Topo Record #12520
Conversation
Signed-off-by: Matt Lord <mattalord@gmail.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
If a new flag is being introduced:
If a workflow is added or modified:
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
Signed-off-by: Matt Lord <mattalord@gmail.com>
bf67af0
to
ce34f61
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
baf15a0
to
b4c347f
Compare
We should not ever make an assumption about how quickly an operation will take. Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this! I think the main question (also found inline): if we update SrvKeyspace
, and that in turn updates Keyspace
, does the change propagate back down to all other SrvKeyspace
s? Should it? Let's discuss.
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've applied a few minor suggestions (remove excessive waits in testing; use context for timeout) -- let's merge this! (assuming tests are good)
Both the test failures are flaky and there are people looking into them. |
I applied the |
…#12520) (#12576) * Store throttler config in global Keyspace record Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix unit test after ThrottlerConfig in Keyspace Signed-off-by: Matt Lord <mattalord@gmail.com> * Apply changes lost in cherry picks Signed-off-by: Matt Lord <mattalord@gmail.com> * Always wait for a status before the final check. We should not ever make an assumption about how quickly an operation will take. Signed-off-by: Matt Lord <mattalord@gmail.com> * Propagate PartialResult and other errs back to client Signed-off-by: Matt Lord <mattalord@gmail.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vtctlutil.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vtgate_util.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * fix grammar Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Co-authored-by: Matt Lord <mattalord@gmail.com> Co-authored-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
* Move to Go 1.19 atomics (vitessio#12391) * Move to Go 1.19 atomics This change moves to Go 1.19 atomics in favor of the version we had in the `sync2` package. The version in that package predates the availability of the new atomics, but for the future we want to use the Go provided ones. On top of that, we can also use the semaphore from `x/sync` instead of a custom one in `sync2`. This cleans up a bunch of additional code from `sync2`. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Don't assume the value is a duration This can be any float, so we need to store the float bits in an atomic uint64 instead (as Go has no atomic floats). Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Log actual error if acquiring semaphore fails Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix overlooked metrics threshold case Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: Matt Lord <mattalord@gmail.com> * [release-16.0] Throttler: Store Config in Global Keyspace Topo Record (vitessio#12520) (vitessio#12576) * Store throttler config in global Keyspace record Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix unit test after ThrottlerConfig in Keyspace Signed-off-by: Matt Lord <mattalord@gmail.com> * Apply changes lost in cherry picks Signed-off-by: Matt Lord <mattalord@gmail.com> * Always wait for a status before the final check. We should not ever make an assumption about how quickly an operation will take. Signed-off-by: Matt Lord <mattalord@gmail.com> * Propagate PartialResult and other errs back to client Signed-off-by: Matt Lord <mattalord@gmail.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vtctlutil.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Update go/test/endtoend/onlineddl/vtgate_util.go Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * fix grammar Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Co-authored-by: Matt Lord <mattalord@gmail.com> Co-authored-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Flakes: Use new healthy shard check in vreplication e2e tests (vitessio#12502) * Use new healthy shard check in vreplication e2e tests This is needed because checking that there's a primary tablet for the shard in vtgate's healtcheck is no longer a reliable indicator that the shard has a healthy serving primary, because now a primary needs to initialize its sidecar database and wait for that to replicate via semi-sync before it becomes serving and can proceed to perform normal functions. So this delay could cause test flakiness if you required a healthy shard before continuing with the test. Signed-off-by: Matt Lord <mattalord@gmail.com> * Try to address unit test race flakes around log size They looked like this: WARNING: DATA RACE Write at 0x000005bf9b60 by goroutine 27141: github.com/spf13/pflag.newUint64Value() /home/runner/go/pkg/mod/github.com/spf13/pflag@v1.0.5/uint64.go:9 +0x5a github.com/spf13/pflag.(*FlagSet).Uint64Var() /home/runner/go/pkg/mod/github.com/spf13/pflag@v1.0.5/uint64.go:45 +0x55 vitess.io/vitess/go/vt/log.RegisterFlags() /home/runner/work/vitess/vitess/go/vt/log/log.go:81 +0x64 vitess.io/vitess/go/vt/servenv.GetFlagSetFor() /home/runner/work/vitess/vitess/go/vt/servenv/servenv.go:347 +0x183 vitess.io/vitess/go/vt/servenv.ParseFlags() /home/runner/work/vitess/vitess/go/vt/servenv/servenv.go:326 +0x49 ... Previous read at 0x000005bf9b60 by goroutine 27136: 1744 github.com/golang/glog.(*syncBuffer).Write() ... And they most often occurred in the wrangler unit tests, which makes sense because it creates a log of loggers. Signed-off-by: Matt Lord <mattalord@gmail.com> * Revert "Try to address unit test race flakes around log size" This reverts commit 51992b8. Signed-off-by: Matt Lord <mattalord@gmail.com> * Use external cluster vtctld in TestMigrate Signed-off-by: Matt Lord <mattalord@gmail.com> * Use subshell vs command output interpolation Signed-off-by: Matt Lord <mattalord@gmail.com> * Ingnore any config files in mysql alias Signed-off-by: Matt Lord <mattalord@gmail.com> --------- Signed-off-by: Matt Lord <mattalord@gmail.com> * Support Custom SidecarDB Names on VTTablets (vitessio#12240) * WIP Signed-off-by: Matt Lord <mattalord@gmail.com> * vreplication package Signed-off-by: Matt Lord <mattalord@gmail.com> * Revert OnlineDDL package changes for now Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix unit tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix InitShardPrimary / ReparentJournal Signed-off-by: Matt Lord <mattalord@gmail.com> * onlineddl package Signed-off-by: Matt Lord <mattalord@gmail.com> * Shorten/improve exported func name Signed-off-by: Matt Lord <mattalord@gmail.com> * Remaining work Signed-off-by: Matt Lord <mattalord@gmail.com> * Remove redundancy from exported function names Signed-off-by: Matt Lord <mattalord@gmail.com> * Minor changes after self review Signed-off-by: Matt Lord <mattalord@gmail.com> * More testing, more fixes Signed-off-by: Matt Lord <mattalord@gmail.com> * Move from vttablet flag to topo Keyspace record Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix vtctld API unit test Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix TestHelp Signed-off-by: Matt Lord <mattalord@gmail.com> * Update throttler service initialization Signed-off-by: Matt Lord <mattalord@gmail.com> * Use custom sidecar in VReplication e2e tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Make sidecardb name concurrecy safe Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix VDiff2 e2e test and remove other hardecoded sidecar names Signed-off-by: Matt Lord <mattalord@gmail.com> * Adjust throttler initialization Signed-off-by: Matt Lord <mattalord@gmail.com> * Post merge fixes Signed-off-by: Matt Lord <mattalord@gmail.com> * Copy SidecarDBName from global Keyspace object to SrvKeyspace objects This way it can be easily accessed by vtgate, vtctld, etc via their existing ResilentServer SrvKeyspace watchers in their own local cell. Signed-off-by: Matt Lord <mattalord@gmail.com> * Store throttler config in global Keyspace record Signed-off-by: Matt Lord <mattalord@gmail.com> * Update vtadmin protos Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix unit test after ThrottlerConfig in Keyspace Signed-off-by: Matt Lord <mattalord@gmail.com> * SidecarDbName is not needed in SrvKeyspace records Signed-off-by: Matt Lord <mattalord@gmail.com> * Remove sqlparser.ReplaceTableQualifiers from query hot path. Signed-off-by: Matt Lord <mattalord@gmail.com> * Add topo timeout for SHOW VITESS_MIGRATIONS Signed-off-by: Matt Lord <mattalord@gmail.com> * Cache sidecar database identifiers by keyspace Signed-off-by: Matt Lord <mattalord@gmail.com> * Modify onlineddl package 1. Move sidecardb table qualifier mgmt to execQuery() 2. Move e2e test back to tablet flag based throttler config Signed-off-by: Matt Lord <mattalord@gmail.com> * Revert "Fix unit test after ThrottlerConfig in Keyspace" This reverts commit 303db96. Signed-off-by: Matt Lord <mattalord@gmail.com> * Revert "Store throttler config in global Keyspace record" This reverts commit 1e7c750. Signed-off-by: Matt Lord <mattalord@gmail.com> * Finish ThrottlerTopoConfig change rollback Signed-off-by: Matt Lord <mattalord@gmail.com> * Add max identifier length check for --sidecar-db-name flag Signed-off-by: Matt Lord <mattalord@gmail.com> * Move vdiff and vreplication packages to binlogplayer.DBClient.ExecuteFetch() Signed-off-by: Matt Lord <mattalord@gmail.com> * binlogplayer.DBClient: eliminate overhead when default name is used Signed-off-by: Matt Lord <mattalord@gmail.com> * Move keyspace ID look to a helper function In case it needs to be used elsewhere. Signed-off-by: Matt Lord <mattalord@gmail.com> * Minor cleanup Signed-off-by: Matt Lord <mattalord@gmail.com> * Add context for usage in the local examples Signed-off-by: Matt Lord <mattalord@gmail.com> * Correct CreateKeyspace command's sidecardb flag validation Signed-off-by: Matt Lord <mattalord@gmail.com> * Limit overhead for OnlineDDL package Signed-off-by: Matt Lord <mattalord@gmail.com> * Cleanup and improvements after self review Signed-off-by: Matt Lord <mattalord@gmail.com> * Store each keyspace's sidecar db identifier in schema.Tracker Signed-off-by: Matt Lord <mattalord@gmail.com> * Enable custom sidecar db name in schema tracker e2e tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix schema.tracker and its tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Better support downgrade tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Always use sqlparser.String() for proper escaping Signed-off-by: Matt Lord <mattalord@gmail.com> * Try to fix Upgrade Downgrade Testing Query Serving (Schema) Signed-off-by: Matt Lord <mattalord@gmail.com> * Minor changes after self review. Signed-off-by: Matt Lord <mattalord@gmail.com> * Mark new flag/functionality as experimental Signed-off-by: Matt Lord <mattalord@gmail.com> * Remove exclamation point from example error msg Signed-off-by: Matt Lord <mattalord@gmail.com> * Fixups after merge Signed-off-by: Matt Lord <mattalord@gmail.com> * More post merge fixes Signed-off-by: Matt Lord <mattalord@gmail.com> * Address review comments Signed-off-by: Matt Lord <mattalord@gmail.com> * Moar knits! Signed-off-by: Matt Lord <mattalord@gmail.com> * Moar knits -- the sequel Signed-off-by: Matt Lord <mattalord@gmail.com> * Deflake schematracker e2e tests They have gotten flakier since the tablet init changes. Signed-off-by: Matt Lord <mattalord@gmail.com> * Add a dedicated in-memory cache for sidecar db idents Signed-off-by: Matt Lord <mattalord@gmail.com> * Improve sidecardb identifier cache Signed-off-by: Matt Lord <mattalord@gmail.com> * More minor improvements Signed-off-by: Matt Lord <mattalord@gmail.com> * Minor tweaks after final self review Signed-off-by: Matt Lord <mattalord@gmail.com> * Minor tidy work Signed-off-by: Matt Lord <mattalord@gmail.com> * Delete entry from cache on srvkeyspace deletion Signed-off-by: Matt Lord <mattalord@gmail.com> * Address review comments. Signed-off-by: Matt Lord <mattalord@gmail.com> * Save me from my nitting! (use vtrpcpb convention) Signed-off-by: Matt Lord <mattalord@gmail.com> * Make IdentifierCache more concurrency-safe than it needs to be (today) Signed-off-by: Matt Lord <mattalord@gmail.com> * Slighly improve code coverage Signed-off-by: Matt Lord <mattalord@gmail.com> * Let caller know if New created a cache or returned inited singleton. Signed-off-by: Matt Lord <mattalord@gmail.com> * Address review comments Signed-off-by: Matt Lord <mattalord@gmail.com> * Use more unique name for unit test Signed-off-by: Matt Lord <mattalord@gmail.com> * Address latest review comments Signed-off-by: Matt Lord <mattalord@gmail.com> * Use lower context for updating keyspace Signed-off-by: Matt Lord <mattalord@gmail.com> --------- Signed-off-by: Matt Lord <mattalord@gmail.com> * Adjust version checks in test utils Signed-off-by: Matt Lord <mattalord@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Description
Related Issue(s)
Checklist