vtgate: record warning for partially successful cross-shard commits

[maxenglander]

Description

When using multi transaction mode (the default), it is possible for Vitess to successfully commit to one shard, but fail to commit to a subsequent shard, thus breaking the atomicity of a multi-shard transaction.

This PR updates VTGate to report partial-success commits in warnings, e.g.:

mysql> commit;
ERROR 1317 (70100): target: customer.-80.primary: vttablet: rpc error: code = Aborted desc = transaction 1703182545849001001: ended at 2023-12-21 14:07:41.515 EST (exceeded timeout: 30s) (CallerID: userData1)
mysql> show warnings;
+---------+------+-----------------------------------------------------+
| Level   | Code | Message                                             |
+---------+------+-----------------------------------------------------+
| Warning |  301 | multi-db commit failed after committing to 1 shards |
+---------+------+-----------------------------------------------------+
1 row in set, 1 warning (0.00 sec)

Also increments the vtgate_warnings{type="NonAtomicCommit"} count.

Related Issue(s)

This PR is intended as a partial, approximate solution for #14815.

Checklist

Backport labels not needed.

• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on CI?
• Documentation was added content/en/docs/19.0: vtgate partial shard commit warnings website#1655

[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.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

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

Backward compatibility

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

[harshit-gangal]

The idea behind raising a warning is good. Should we also provide the shards where the commit was successful? So, the users can do manual fixes if needed.

[harshit-gangal]

LGTM, left one comment.

[maxenglander]

@harshit-gangal I'm happy to make that change! What's a reasonable upper bound on the number of shards to report in the warning? 256?

[harshit-gangal]

@harshit-gangal I'm happy to make that change! What's a reasonable upper bound on the number of shards to report in the warning? 256?

256 would be a very large number to report in shards.
We can keep it to 8 or 16.

[harshit-gangal]

You should add this change in release notes here: https://github.com/vitessio/vitess/blob/main/changelog/19.0/19.0.0/summary.md

[maxenglander]

@harshit-gangal added a change log note and limited shards reported in warn to 16.

[harshit-gangal]

can you add a test where this is covered?

[maxenglander]

gladly, added