OnlineDDL: avoid schema_migrations AUTO_INCREMENT gaps by pre-checking for existing migration

[shlomi-noach]

Description

Fixes #12168
This PR prevents excessive gaps in _vt.schema_migrations id values with a small refactor:

• Instead of relying on INSERT IGNORE, we first SELECT for existing migration UUID, and only if it does not exist, we INSERT.
• Because this is now a two-step logic, we add a mutex around the function (the mutex is only used by SubmitMigration function and does not otherwise conflict with the rest of the scheduler's work)
• The logic that used to follow the INSERT IGNORE, which addresses the case where the UUID pre-existed, is now moved to run before INSERT IGNORE

Tests are unchanged. We expect no change in behavior.

Related Issue(s)

#12168

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

Deployment Notes

[shlomi-noach]

converted to draft, doing another necessary refactor (need to take singleron/singleton-context scenario into consideration)

[shlomi-noach]

OK, did another refactor; extracted an inner function, validateSingleton() and some logic around it, onto a new function, func (e *Executor) submitCallbackIfNonConflicting(). And this logic applies to either submitting a migration or retrying a migration. It validates whether it is OK for the given migration to retry/submit given singleton or singleton-context constraints.

[mattlord]

LGTM!

Not breaking any test would indicate that we probably don't have test coverage around submissions. 🙂 I think we should add more unit tests eventually (that could also e.g. confirm that we don't have gaps anymore when we resubmit) but that's a bigger issue and not specific to this work.

In the meantime, can we easily add anything to the endtoend tests to confirm the behavioral change? Perhaps a select at the end of a workflow to look for gaps in the migration IDs?

[shlomi-noach]

Not breaking any test would indicate that we probably don't have test coverage around submissions.

The opposite is true. We have plenty tests around submissions -- every single test, really, and in particular all singleton and singleton-context tests -- literally hundreds.

[shlomi-noach]

Perhaps a select at the end of a workflow to look for gaps in the migration IDs?

I'll look into that.

[mattlord]

Not breaking any test would indicate that we probably don't have test coverage around submissions.

The opposite is true. We have plenty tests around submissions -- every single test, really, and in particular all singleton and singleton-context tests -- literally hundreds.

Yea, I meant code test coverage — i.e. unit tests. 🙂 Was just noting it (not a blocker).

[shlomi-noach]

Added an explicit test to validate id values are sequential and without gaps; this test is now called at the end of multiple endtoend workflows.

[deepthi]

request for review from @vitessio/query-serving