Replication: Have the DB flavor process waiting for a pos

[mattlord]

Description

The different database flavors use different SQL functions in order to perform the generic "wait for the replica to reach this position" functionality — and each function has different semantics for what it returns and what those return values mean.

We have previously only asked the flavor interface to provide the command/query (which leveraged the flavor's specific SQL function) to use and then executed the provided query and processed the results at a higher level such as the mysqlctl package — where the specifics of the flavor implementation's SQL function were not known. Prior to moving to the newer WAIT_UNTIL_SQL_THREAD_AFTER_GTIDS() SQL function for GTID based MySQL replication in #14612, the differences were not significant enough that they caused known problems using this method.

However, the return values on timeout from WAIT_UNTIL_SQL_THREAD_AFTER_GTIDS and WAIT_FOR_EXECUTED_GTID_SET are not the same. The old one returns -1, whereas the new one returns 1. This was causing us to assume that WaitForSourcePos was successful when in fact it had timed out, leading to #14738.

When we tried to fix this in #14739, it proved problematic to try and process the results outside of the flavor implementation given the semantic differences. This PR moves the handling/processing of the WaitForPos functionality down into the flavor interface (as much of the other functionality is, e.g. status), and eliminates the need to make any assumptions about return values in the calling function.

Related Issue(s)

• Fixes: Adapt Vitess to MySQL deprecation of WAIT_UNTIL_SQL_THREAD_AFTER_GTIDS #14611
• Fixes: Bug Report: PRS promotes replica that has not caught to old primary as new primary #14738

Checklist

• "Backport to:" labels have been added if this change should be back-ported
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on the CI
• Documentation was added or is not required

EDIT Dec 11, 2023: Folded in context from previous PR #14739 so that this PR has the full story.

[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.

[GuptaManan100]

LGTM!

[dbussink]

Better isolation this way so that's good!

[shlomi-noach]

Looks good. Thank you for this fix!