VStreamer: fix deadlock when there are a lot of vschema changes as well as db events

[rohit-nayak-ps]

Description

This is an attempt at fixing the bug reported in https://github.com/vitessio/vitess/issues/11169, (see https://github.com/vitessio/vitess/pull/11268 for Matt Lord's PR related to this).

Description TBD

Pre-conditions for bug:

1. There is a vstreamer client that is subscribed to, but not reading events fast enough, from a Replica
2. There are a lot of VSchema changes at this time
3. A PRS happens at this time, switching this Replica to a Primary

Bug cause:

1. In vstreamer.SetVSchema() the new vschema is written to a non-buffered channel for each active vstreamer. This is read by the vstreamer event processor, holding the vstreamer engine mutex.
2. In the vstreamer event processor there are two parallel case statements: one reads from the vschema channel and other reads the binlog events. The binlog events are streamed to the receiver. If the receiver is not reading them then the send will block. This means that the other case which polls the vschema change channel will not get called. This results in the vstreamer engine lock being held until all events are streamed
3. The PRS results in the vttablet state manager trying to shut down the vstreamer engine for which it needs the vstreamer engine mutex. This can cause the PRS to get blocked. (Still need to investigate if certain contexts start getting cancelled which we should be handling. Matt's PR already added more checks for cancelled contexts.)

This PR drains the vschema channel so that only the latest vschema update is sent. So it no longer holds the vstreamer lock for an extended time allowing the PRS to proceed.

The change in https://github.com/vitessio/vitess/pull/11268 also solves the same issue in a different way since the vschema update no longer holds a lock (since it uses atomic.int32 to signal whether the vstreamer engine is open, rather than the mutex).

Signed-off-by: Rohit Nayak [email protected]

Related Issue(s)

Checklist

• [X] "Backport me!" label has been added if this change should be backported
• [X] Tests were added or are not required
• [ ] Documentation was added or is not required

[vitess-bot[bot]]

Review Checklist

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

General

• [x] Ensure that the Pull Request has a descriptive title.
• [x] 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.
• [x] If a new flag is being introduced, review whether it is really needed. The flag names should be clear and intuitive (as far as possible), and the flag's help should be descriptive. Additionally, flag names should use dashes (-) as word separators rather than underscores (_).
• [x] If a workflow is added or modified, each items in Jobs should be named in order to mark it as required. If the workflow should be required, the GitHub Admin should be notified.

Bug fixes

• [x] There should be at least one unit or end-to-end test.
• [x] The Pull Request description should either include a link to an issue that describes the bug OR an actual description of the bug and how to reproduce, along with a description of the fix.

Non-trivial changes

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

New/Existing features

• [x] Should be documented, either by modifying the existing documentation or creating new documentation.
• [x] 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

• [x] Protobuf changes should be wire-compatible.
• [x] Changes to _vt tables and RPCs need to be backward compatible.
• [x] vtctl command output order should be stable and awk-able.