Enable mtr retries in MariaDB CI integration - #3427
Open
justsmth wants to merge 1 commit into
Open
Conversation
run_mariadb_integration.sh passed --retry-failure=2 but never set --retry,
which defaults to 1 in mariadb-test-run.pl. The retry gate reads
my $retries= $result->{retries} || 2;
if ($test_has_failed and $retries <= $opt_retry){
so on a first failure the counter evaluates to 2, "2 <= 1" is false, and no
retry is ever attempted. --retry-failure only caps retries, so on its own it
was dead. --force does not enable retries either: the one place $opt_retry is
reassigned clamps it back to 1 under --debug.
Set --retry=2 so a failed test gets a single retry. That is enough to absorb
the order-dependent failures this suite produces. main/ps_change_master.test
ends by re-pointing MASTER_HOST instead of issuing RESET SLAVE ALL, which
leaves mysqld configured as a replica; a later main.start_slave_until landing
on the same worker then sees START SLAVE UNTIL succeed rather than fail with
ER_BAD_SLAVE. A retry is dispatched back to the worker that just failed, and
that worker has already run stop_all_servers() via report_failure_and_restart(),
so its next run_testcase() calls clean_datadir() and reinstalls from scratch.
The retried attempt starts with no master.info and passes.
Contributor
|
🔒 Security Review — View Report Please review before merging. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3427 +/- ##
==========================================
- Coverage 78.24% 78.23% -0.01%
==========================================
Files 696 696
Lines 124511 124511
Branches 17279 17278 -1
==========================================
- Hits 97418 97413 -5
- Misses 26168 26175 +7
+ Partials 925 923 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
prasden
approved these changes
Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes:
run_mariadb_integration.shpasses--retry-failure=2but never sets--retry, which defaults to1("no retries") inmariadb-test-run.pl. The retry branch is therefore unreachable and--retry-failure, which only caps retries, has nothing to cap -- so the flag we already had was a no-op, and any single flaky test failed the whole job.--forcedoesn't enable retries either. This sets--retry=2so a failed test gets one retry, plus a short comment so the pairing doesn't look redundant to a future reader.Call-outs:
Retrying beats skipping more tests here because a retried attempt is guaranteed a clean server, not merely a likely-clean one:
mtrdispatches the retry back to the same worker that just failed, and that worker has already runstop_all_servers()viareport_failure_and_restart(), so its nextrun_testcase()hitsclean_datadir()and reinstalls from scratch. That covers the order-dependent failures this suite produces, where one test leaks server state into a later one on the same worker -- the recent case beingps_change_master.testending inCHANGE MASTER TO MASTER_HOST=...rather thanRESET SLAVE ALL, leavingmysqldconfigured as a replica so a latermain.start_slave_untilnever got its expectedER_BAD_SLAVE. Whether it trips depends on--parallel=autoworker fan-out, which is why aarch64 failed while x86-64 passed in the same run.--retry-failure=2is now unreachable in practice, since the cap is only consulted after a second failure and--retry=2never allows one. Keeping it anyway: it's already the documented default and it bounds things explicitly if--retryis ever raised.This doesn't fix the upstream isolation bug, only stops it failing our CI. Reporting that separately.
Testing:
Existing CI -- the
mariadb-x86_64andmariadb-aarch64jobs exercise this directly, though a green run won't demonstrate the retry itself since the behavior only appears when a test fails. Locally:bash -n, plus traced the retry, shutdown, and datadir-cleanup paths inmariadb-test-run.platMariaDB/server@main, the revision this job shallow-clones.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.