SidecarDB Init: don't fail on schema init errors

[rohit-nayak-ps]

Description

The initial implementation of the new mechanism for initializing the sidecar database in #11520 causes a failure if there is an error while applying a DDL to create or alter even a single sidecardb table. Such a failure causes the tablet initialization to fail. Hence if there is an issue with one of the queries to create or upgrade a table, say due to an incompatible MySQL version we would end up with tablets not reaching the serving state.

Now, if this happens during a Vitess version upgrade, it would not be catastrophic because the upgrade process would be stopped when the first replica being upgraded fails to launch.

However if, for any reason, the entire cluster rolls over to a new Vitess version in the presence of these failures, we would have a catastrophic incident where we could not serve queries. This can also happen if we upgrade the MySQL version on all tablets for the same Vitess version.

For example, we recently encountered a user cluster where a schema upgrade (using withddl) had caused a cluster to fail when the underlying MySQL was upgraded. This was due to a data inconsistency: one of the sidecar tables had zero dates and a table alter which added an unrelated column failed on newer versions of MySQL.

While such issues might happen very rarely, the possible existence of such critical issues is a strong reason not to fail tablet launches if we encounter errors during the sidecar init process.

This PR ignores errors in the sidecardb module when we execute create or alter queries on sidecardb tables. Quietly failing errors or just logging is also not ideal since it means some modules or some functionality of modules can fail due to an incorrect schema of a table used by that module and debugging them can be time consuming (Note that we had the same issue the withDDL approach).

To surface these issues we have added two stats which are available via the debug/vars endpoint:

• SidecarDBDDLErrorCount which is the number of errors encountered
• SidecarDBDDLErrors which is a list of the corresponding golang errors

A future PR will add functionality to VTAdmin to create visual alerts based on these stats.

Related Issue(s)

#10133

Checklist

• "Backport to:" labels have been added if this change should be back-ported
• Tests were added or are not required
• Documentation was added or is not required

[vitess-bot]

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.

[deepthi]

Needs a proper description and link to tracking issue / previous PR.

[rohit-nayak-ps]

Needs a proper description and link to tracking issue / previous PR.

Duh, sorry, was multi-tasking and forgot to save the description in another tab.

[frouioui]

Should this key be a constant? I see we are using it in several places.

[rohit-nayak-ps]

Fixed

[mattlord]

I only had some minor nits and suggestions. I will let you make the final call on those.

[mattlord]

Nit, but DDL is an initialism and the letters should be capitalized to reflect that (even though I think all readers know what DDL is).

[mattlord]

To be safe, we should check that it was a valid cast. We don't want to crash the vttablet process for new error handling.

[mattlord]

Same note here about additional safety by checking for a valid cast.

[mattlord]

Same note here about capitalizing DDL.

[mattlord]

IMO it's worth avoiding this imprecise string matching. We could instead parse it, walk the AST, have a case where the type is not SHOW, then if the table identifier == the table name in the test we return the error. I can help with this if you want.