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

Support Custom SidecarDB Names on VTTablets #12240

Merged
merged 84 commits into from
Mar 14, 2023

Conversation

mattlord
Copy link
Contributor

@mattlord mattlord commented Feb 6, 2023

Description

This PR adds a new experimental feature: keyspace level sidecar database names. For the why, please see the feature request.

In this PR the sidecar db name can only be set when the keyspace is first created via the CreateKeyspace vtctl client command using the new --sidecar-db-name command flag. From that point forward it is immutable (w/o modifying the topo directly outside of Vitess). If no value is specified for the keyspace then it will be set to the default of _vt when the keyspace is created, and as a fallback the default will also be set when the first tablet is started within the keyspace if for any reason it's not already set in the topo keyspace record.

The management of the sidecar database resides entirely within a vttablet. This allows each vttablet in a keyspace to manage its own sidecar database independently of all other keyspaces and processes in the cluster. This allows e.g. for most tablets within a Vitess cluster to use the default sidecar name of _vt while some can use a non-standard name of e.g. _vt_import or __virtual_vitess_cluster for whatever purposes as needed (in various cases the tablet's mysqld instance may be participating in two distinct vitess clusters or two distinct keyspaces). From outside of the vttablet, Vitess is able to assume that the standard name of _vt is being used (including external/3rd party tooling that e.g. may be doing things like vtctlclient ExecuteFetchAsDba) unless talking directly to a tablet's mysqld instance.

Note there are two exceptions to the above rule today — where another process does need to know the sidecar database name that a vttablet is using (by way of the keyspace it's in). Both are in vtgate and are due to the fact that today vtgate sends raw SQL queries to a vttablet through the normal user query hot path (tabletserver's QueryExecutor.Execute()) in the following cases:

  1. OnlineDDL's show vitess_migrations implementation
  2. VTGate's Schema Tracker implementation

The plan is to eliminate these two exceptions in the future by leveraging new vttablet RPCs that vtgate can then use instead of the current SQL interface. This way we have proper encapsulation and vtgate does not need to be aware of the implementation details (in this case the schema vttablet uses for the storage).

Related Issue(s)

Checklist

Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
@vitess-bot vitess-bot bot added the NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work label Feb 6, 2023
@vitess-bot
Copy link
Contributor

vitess-bot bot commented Feb 6, 2023

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

  • Ensure that the Pull Request has a descriptive title.
  • If this is a change that users need to know about, please apply the release notes (needs details) label so that merging is blocked unless the summary release notes document is included.
  • If a test is added or modified, there should be a documentation on top of the test to explain what the expected behavior is what the test does.

If a new flag is being introduced:

  • Is it really necessary to add this flag?
  • Flag names should be clear and intuitive (as far as possible)
  • Help text should be descriptive.
  • Flag names should use dashes (-) as word separators rather than underscores (_).

If a workflow is added or modified:

  • Each item in Jobs should be named in order to mark it as required.
  • If the workflow should be required, the maintainer team should be notified.

Bug fixes

  • There should be at least one unit or end-to-end test.
  • The Pull Request description should include a link to an issue that describes the bug.

Non-trivial changes

  • There should be some code comments as to why things are implemented the way they are.

New/Existing features

  • Should be documented, either by modifying the existing documentation or creating new documentation.
  • New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

  • Protobuf changes should be wire-compatible.
  • Changes to _vt tables and RPCs need to be backward compatible.
  • vtctl command output order should be stable and awk-able.
  • RPC changes should be compatible with vitess-operator
  • If a flag is removed, then it should also be removed from VTop, if used there.

@vitess-bot vitess-bot bot added the NeedsWebsiteDocsUpdate What it says label Feb 6, 2023
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord force-pushed the custom_sidecar_db branch 2 times, most recently from dd1e53c to a9689e1 Compare February 12, 2023 22:15
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord force-pushed the custom_sidecar_db branch 2 times, most recently from 751f0fa to 8d17eb4 Compare February 13, 2023 01:30
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord force-pushed the custom_sidecar_db branch 2 times, most recently from 45d4475 to fe7e759 Compare February 13, 2023 23:48
Signed-off-by: Matt Lord <mattalord@gmail.com>
Copy link
Member

@harshit-gangal harshit-gangal left a comment

Choose a reason for hiding this comment

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

some nits, mostly looks good to me.

Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord
Copy link
Contributor Author

mattlord commented Mar 14, 2023

some nits, mostly looks good to me.

Thanks for the helpful comments @harshit-gangal ! I believe that I've addressed all of them here: da4ee79

And in a follow-up: 2c11e74

Signed-off-by: Matt Lord <mattalord@gmail.com>
@mattlord mattlord merged commit 1d18df4 into vitessio:main Mar 14, 2023
@mattlord mattlord deleted the custom_sidecar_db branch March 14, 2023 17:52
mattlord added a commit to vitessio/website that referenced this pull request Mar 15, 2023
This documents the work done in:
  vitessio/vitess#12240

Signed-off-by: Matt Lord <mattalord@gmail.com>
mattlord added a commit to vitessio/website that referenced this pull request Mar 15, 2023
This documents the work done in:
  vitessio/vitess#12240

Signed-off-by: Matt Lord <mattalord@gmail.com>
mattlord added a commit to vitessio/website that referenced this pull request Mar 15, 2023
This documents the work done in:
  vitessio/vitess#12240

Signed-off-by: Matt Lord <mattalord@gmail.com>
mattlord added a commit to vitessio/website that referenced this pull request Mar 20, 2023
This documents the work done in:
  vitessio/vitess#12240

Signed-off-by: Matt Lord <mattalord@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Support Custom/Variable Sidecar (_vt) Database Name on Tablets
5 participants