fix: resolve CI failures for MariaDB, Percona, ts tests, fan-in#79
fix: resolve CI failures for MariaDB, Percona, ts tests, fan-in#79renecannao merged 2 commits intomasterfrom
Conversation
…n-in - innodb_cluster.go: remove dead makeNodesList() call (result immediately overwritten) - downloads/tarball_list.json: replace all MariaDB entries with archive.mariadb.org URLs using systemd filename pattern; verified versions 10.6.9, 10.11.9, 11.4.5, 11.4.9, 11.8.6 - integration_tests.yml: update MariaDB matrix to 10.11.9 + 11.4.5 and use archive.mariadb.org with systemd tarball name - integration_tests.yml: try glibc2.35 first for Percona (works on ubuntu-22.04), fall back to glibc2.17 - integration_tests.yml: remove MySQL 5.7.44 from ts test (minimal tarball URL broken), use 8.0/8.4/9.5 only - integration_tests.yml: add sleep 5 after PostgreSQL multiple deploy to avoid pg_ctl start timeout - fan-in.tmpl: add sleep 3 before test_replication to allow replication catchup
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpdate CI workflows, tarball registry, and test templates: Percona glibc default changed to glibc2.35 with glibc2.17 fallback; MariaDB downloads shifted to archive.mariadb.org with version matrix adjustments; MySQL 5.7 removed from test matrix; extra sleeps added in replication and deployment tests; minor InnoDBCluster initialization simplification. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the MariaDB tarball list by replacing several versions with archived systemd-compatible versions and simplifies the initialization of slaveList in sandbox/innodb_cluster.go. Additionally, a sleep 3 command was added to the fan-in.tmpl test template. Feedback suggests replacing this fixed sleep with a more robust polling mechanism to avoid test flakiness.
| [version_is_at_least:$db_version:8.0.0] env required_ports=6 | ||
| check_ports $sb_dir $required_ports | ||
|
|
||
| sleep 3 |
There was a problem hiding this comment.
Using a fixed sleep can make tests flaky. If the system is under load, 3 seconds may not be enough for replication to catch up, causing the test to fail. Conversely, on a fast system, this adds an unnecessary delay.
A more robust solution would be to poll for the replication status. Since the test-scripting language doesn't seem to support loops, you could create a helper script (e.g., in shell) that waits for replication to be caught up.
For example, a wait-for-replication.sh script could be:
#!/bin/bash
SB_DIR=$1
TIMEOUT=30
for i in $(seq 1 $TIMEOUT); do
# This command will return a non-empty string if any channel has a non-zero or NULL lag.
LAGGING_CHANNELS=$($SB_DIR/n3 -e "SHOW SLAVE STATUS\G" | grep 'Seconds_Behind_Master:' | grep -v ': 0$')
if [ -z "$LAGGING_CHANNELS" ]; then
echo "Replication caught up."
exit 0
fi
sleep 1
done
echo "Timeout waiting for replication to catch up."
exit 1Then you could call this script from the test template instead of using sleep. This would make the test more reliable and efficient.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/integration_tests.yml (1)
406-407: Consider adding a readiness check instead of a fixed sleep.The
sleep 5is a pragmatic fix for pg_ctl start timeouts, but fixed delays can be flaky—too short on slow CI runners or unnecessarily long on fast ones. A polling loop that checks node readiness (e.g.,pg_isready) would be more robust.That said, the current fix addresses the immediate CI failure and can be improved later.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/integration_tests.yml around lines 406 - 407, Replace the fixed delay caused by the "sleep 5" after the echo "=== Waiting for nodes to start ===" with a polling readiness check (e.g., use pg_isready in a loop or test TCP port 5432) that retries until success or a sensible timeout; implement a loop that calls pg_isready for each node, waits a short interval between attempts, and exits with failure if the overall timeout is reached to avoid indefinite waits.downloads/tarball_list.json (1)
3306-3370: No aarch64 entries for new MariaDB versions.The new MariaDB entries only include x86_64 architecture. If arm64/aarch64 builds are available on
archive.mariadb.org, consider adding them for broader platform support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@downloads/tarball_list.json` around lines 3306 - 3370, The new MariaDB JSON entries only include x86_64 builds (e.g., name "mariadb-11.8.6-linux-systemd-x86_64.tar.gz"); add matching aarch64/arm64 entries for each new version if archive.mariadb.org provides them by duplicating each object and updating "name" and "url" to the aarch64 variant (replace "_x86_64" with "_aarch64" or the site’s canonical arm64 filename), set "arch" to "arm64" (or "aarch64" to match existing convention), and fill in "size" and "date_added"/"notes" to match the x86 entries; ensure entries for mariadb-10.6.9, 10.11.9, 11.4.5, 11.4.9, and 11.8.6 are added with correct URLs and metadata.ts/templates/multi-source/fan-in.tmpl (1)
36-39: Consider applying the same fix toall-masters.tmpl.The
sleep 3addition in fan-in.tmpl addresses timing issues where replication hasn't caught up beforetest_replicationruns. However,ts/templates/multi-source/all-masters.tmplhas the same execution pattern—check_portsfollowed byexec test_replication—but lacks this sleep.If the sleep is necessary for fan-in topology's replication to stabilize, the all-masters topology with its multi-node synchronization likely faces the same potential race condition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ts/templates/multi-source/fan-in.tmpl` around lines 36 - 39, The all-masters.tmpl likely has the same race where replication hasn't caught up before running test_replication; update that template to mirror fan-in.tmpl by inserting a short delay (e.g., sleep 3) between the check_ports step and the exec $HOME/.../test_replication invocation so test_replication runs after replication stabilizes; locate the sequence with check_ports and test_replication in all-masters.tmpl and add the sleep call immediately before the exec test_replication command.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@downloads/tarball_list.json`:
- Around line 3306-3370: The five MariaDB tarball entries
(mariadb-10.6.9-linux-systemd-x86_64.tar.gz,
mariadb-10.11.9-linux-systemd-x86_64.tar.gz,
mariadb-11.4.5-linux-systemd-x86_64.tar.gz,
mariadb-11.4.9-linux-systemd-x86_64.tar.gz,
mariadb-11.8.6-linux-systemd-x86_64.tar.gz) are missing the checksum field and
will be rejected by the registrar check that inspects tb.Checksum (and exempts
only tb.Flavor == "tidb"); download each tarball, compute a strong digest
(prefer SHA512 via sha512sum), then add a checksum property in the JSON for each
entry in the form "checksum": "SHA512:CHECKSUM_VALUE" (or "MD5:..." if you
must), ensuring the checksum string exactly matches the format expected by the
validator.
---
Nitpick comments:
In @.github/workflows/integration_tests.yml:
- Around line 406-407: Replace the fixed delay caused by the "sleep 5" after the
echo "=== Waiting for nodes to start ===" with a polling readiness check (e.g.,
use pg_isready in a loop or test TCP port 5432) that retries until success or a
sensible timeout; implement a loop that calls pg_isready for each node, waits a
short interval between attempts, and exits with failure if the overall timeout
is reached to avoid indefinite waits.
In `@downloads/tarball_list.json`:
- Around line 3306-3370: The new MariaDB JSON entries only include x86_64 builds
(e.g., name "mariadb-11.8.6-linux-systemd-x86_64.tar.gz"); add matching
aarch64/arm64 entries for each new version if archive.mariadb.org provides them
by duplicating each object and updating "name" and "url" to the aarch64 variant
(replace "_x86_64" with "_aarch64" or the site’s canonical arm64 filename), set
"arch" to "arm64" (or "aarch64" to match existing convention), and fill in
"size" and "date_added"/"notes" to match the x86 entries; ensure entries for
mariadb-10.6.9, 10.11.9, 11.4.5, 11.4.9, and 11.8.6 are added with correct URLs
and metadata.
In `@ts/templates/multi-source/fan-in.tmpl`:
- Around line 36-39: The all-masters.tmpl likely has the same race where
replication hasn't caught up before running test_replication; update that
template to mirror fan-in.tmpl by inserting a short delay (e.g., sleep 3)
between the check_ports step and the exec $HOME/.../test_replication invocation
so test_replication runs after replication stabilizes; locate the sequence with
check_ports and test_replication in all-masters.tmpl and add the sleep call
immediately before the exec test_replication command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2ed0304-5981-4834-8d6c-d80420096c65
📒 Files selected for processing (4)
.github/workflows/integration_tests.ymldownloads/tarball_list.jsonsandbox/innodb_cluster.gots/templates/multi-source/fan-in.tmpl
| { | ||
| "name": "mariadb-10.6.9-linux-systemd-x86_64.tar.gz", | ||
| "OS": "Linux", | ||
| "arch": "amd64", | ||
| "url": "https://archive.mariadb.org/mariadb-10.6.9/bintar-linux-systemd-x86_64/mariadb-10.6.9-linux-systemd-x86_64.tar.gz", | ||
| "flavor": "mariadb", | ||
| "minimal": false, | ||
| "size": 0, | ||
| "short_version": "10.6", | ||
| "version": "10.6.9", | ||
| "notes": "archive.mariadb.org LTS", | ||
| "date_added": "2026-04-02 00:00" | ||
| }, | ||
| { | ||
| "name": "mariadb-10.11.9-linux-systemd-x86_64.tar.gz", | ||
| "OS": "Linux", | ||
| "arch": "amd64", | ||
| "url": "https://archive.mariadb.org/mariadb-10.11.9/bintar-linux-systemd-x86_64/mariadb-10.11.9-linux-systemd-x86_64.tar.gz", | ||
| "flavor": "mariadb", | ||
| "minimal": false, | ||
| "size": 0, | ||
| "short_version": "10.11", | ||
| "version": "10.11.9", | ||
| "notes": "archive.mariadb.org LTS", | ||
| "date_added": "2026-04-02 00:00" | ||
| }, | ||
| { | ||
| "name": "mariadb-11.4.5-linux-systemd-x86_64.tar.gz", | ||
| "OS": "Linux", | ||
| "arch": "amd64", | ||
| "url": "https://archive.mariadb.org/mariadb-11.4.5/bintar-linux-systemd-x86_64/mariadb-11.4.5-linux-systemd-x86_64.tar.gz", | ||
| "flavor": "mariadb", | ||
| "minimal": false, | ||
| "size": 0, | ||
| "short_version": "11.4", | ||
| "version": "11.4.5", | ||
| "notes": "archive.mariadb.org LTS", | ||
| "date_added": "2026-04-02 00:00" | ||
| }, | ||
| { | ||
| "name": "mariadb-11.4.9-linux-systemd-x86_64.tar.gz", | ||
| "OS": "Linux", | ||
| "arch": "amd64", | ||
| "url": "https://archive.mariadb.org/mariadb-11.4.9/bintar-linux-systemd-x86_64/mariadb-11.4.9-linux-systemd-x86_64.tar.gz", | ||
| "flavor": "mariadb", | ||
| "minimal": false, | ||
| "size": 0, | ||
| "short_version": "11.4", | ||
| "version": "11.4.9", | ||
| "notes": "archive.mariadb.org LTS", | ||
| "date_added": "2026-04-02 00:00" | ||
| }, | ||
| { | ||
| "name": "mariadb-11.8.6-linux-systemd-x86_64.tar.gz", | ||
| "OS": "Linux", | ||
| "arch": "amd64", | ||
| "url": "https://archive.mariadb.org/mariadb-11.8.6/bintar-linux-systemd-x86_64/mariadb-11.8.6-linux-systemd-x86_64.tar.gz", | ||
| "flavor": "mariadb", | ||
| "minimal": false, | ||
| "size": 0, | ||
| "short_version": "11.8", | ||
| "version": "11.8.6", | ||
| "notes": "archive.mariadb.org short-term", | ||
| "date_added": "2026-04-02 00:00" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify MariaDB archive URLs exist and are accessible
URLS=(
"https://archive.mariadb.org/mariadb-10.6.9/bintar-linux-systemd-x86_64/mariadb-10.6.9-linux-systemd-x86_64.tar.gz"
"https://archive.mariadb.org/mariadb-10.11.9/bintar-linux-systemd-x86_64/mariadb-10.11.9-linux-systemd-x86_64.tar.gz"
"https://archive.mariadb.org/mariadb-11.4.5/bintar-linux-systemd-x86_64/mariadb-11.4.5-linux-systemd-x86_64.tar.gz"
"https://archive.mariadb.org/mariadb-11.4.9/bintar-linux-systemd-x86_64/mariadb-11.4.9-linux-systemd-x86_64.tar.gz"
"https://archive.mariadb.org/mariadb-11.8.6/bintar-linux-systemd-x86_64/mariadb-11.8.6-linux-systemd-x86_64.tar.gz"
)
for url in "${URLS[@]}"; do
echo "Checking: $url"
status=$(curl -sI -o /dev/null -w "%{http_code}" "$url" 2>/dev/null)
if [ "$status" = "200" ] || [ "$status" = "302" ]; then
echo " ✓ OK (HTTP $status)"
else
echo " ✗ FAILED (HTTP $status)"
fi
doneRepository: ProxySQL/dbdeployer
Length of output: 774
🏁 Script executed:
# Check the tarball_list.json structure for checksums and sizes
# Get a sample of entries to understand the pattern
head -c 5000 downloads/tarball_list.json | tail -c 2000Repository: ProxySQL/dbdeployer
Length of output: 2063
🏁 Script executed:
# Search for entries with checksums to understand the pattern
rg -A 10 '"checksum"' downloads/tarball_list.json | head -50Repository: ProxySQL/dbdeployer
Length of output: 2071
🏁 Script executed:
# Check if other MariaDB entries (if any) in the file have checksums
rg -B 5 -A 5 'archive.mariadb.org' downloads/tarball_list.jsonRepository: ProxySQL/dbdeployer
Length of output: 2356
🏁 Script executed:
# Check how many entries have size: 0 vs populated sizes
rg '"size": 0' downloads/tarball_list.json | wc -l
rg '"size": [1-9]' downloads/tarball_list.json | head -5Repository: ProxySQL/dbdeployer
Length of output: 188
🏁 Script executed:
# Search for code that validates checksums
rg -i 'checksum' --type go --type js --type py | head -20Repository: ProxySQL/dbdeployer
Length of output: 1848
🏁 Script executed:
# Look for how the tarball_list.json is loaded and used
rg 'tarball_list' --type go -A 5 | head -40Repository: ProxySQL/dbdeployer
Length of output: 831
🏁 Script executed:
# Check if there's checksum validation in the download logic
rg -i 'sha512|sha256|md5|integrity|verify' --type go | grep -i 'download\|checksum' | head -15Repository: ProxySQL/dbdeployer
Length of output: 1600
🏁 Script executed:
# Get the full validation function to understand the impact
rg -A 20 'if tb.Checksum == "" && tb.Flavor != "tidb"' downloads/remote_registry.goRepository: ProxySQL/dbdeployer
Length of output: 792
🏁 Script executed:
# Check if there's checksum validation at load time
rg -B 5 -A 10 'ValidateTarballRegistry\|CheckTarballRegistry' downloads/remote_registry.go | head -50Repository: ProxySQL/dbdeployer
Length of output: 45
🏁 Script executed:
# Verify if archive.mariadb.org provides checksum files
# Try to fetch the SHA256SUM file for one of the versions
curl -sI https://archive.mariadb.org/mariadb-10.6.9/bintar-linux-systemd-x86_64/SHA256SUMS 2>&1 | head -10
curl -sI https://archive.mariadb.org/mariadb-10.6.9/bintar-linux-systemd-x86_64/sha256sum.txt 2>&1 | head -10Repository: ProxySQL/dbdeployer
Length of output: 319
Checksums are required and missing—these entries will fail validation.
The five MariaDB entries cannot be used without checksums. The tarball registry validation code explicitly rejects entries with missing checksums for all flavors except tidb (mariadb entries are not exempted):
if tb.Checksum == "" && tb.Flavor != "tidb" {
tarballErrorList = append(tarballErrorList, tarballError{tb.Name, "checksum is missing"})
}You must populate the checksum field using the format "SHA512:CHECKSUM_VALUE" or "MD5:CHECKSUM_VALUE". Note that archive.mariadb.org does not provide standard checksum files (SHA256SUMS, sha256sum.txt both return 404), so you may need to download the tarballs and compute their checksums locally using sha512sum, or check if MariaDB provides checksums through an alternate method.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@downloads/tarball_list.json` around lines 3306 - 3370, The five MariaDB
tarball entries (mariadb-10.6.9-linux-systemd-x86_64.tar.gz,
mariadb-10.11.9-linux-systemd-x86_64.tar.gz,
mariadb-11.4.5-linux-systemd-x86_64.tar.gz,
mariadb-11.4.9-linux-systemd-x86_64.tar.gz,
mariadb-11.8.6-linux-systemd-x86_64.tar.gz) are missing the checksum field and
will be rejected by the registrar check that inspects tb.Checksum (and exempts
only tb.Flavor == "tidb"); download each tarball, compute a strong digest
(prefer SHA512 via sha512sum), then add a checksum property in the JSON for each
entry in the form "checksum": "SHA512:CHECKSUM_VALUE" (or "MD5:..." if you
must), ensuring the checksum string exactly matches the format expected by the
validator.
…ce ts tests MySQL 8.0.22+ renamed Slave_IO_Running to Replica_IO_Running and Slave_SQL_Running to Replica_SQL_Running. The ts test templates for fan-in and all-masters used hardcoded old names, failing on 8.0.34+. Fixed with regex patterns matching both: (Slave|Replica)_IO_Running
Fixes
MariaDB registry:
downloads.mariadb.comCDN no longer serves tarballs. Switched toarchive.mariadb.orgwith correctsystemdfilename pattern. Real versions: 10.6.9, 10.11.9, 11.4.5, 11.4.9, 11.8.6.MariaDB CI: Updated matrix to 10.11.9/11.4.5, fixed download URL.
Percona CI: Try glibc2.35 first (ubuntu-22.04 compatible), fall back to glibc2.17. Fixes
libssl.so.10 not founderror.Lint: Removed dead
makeNodesListcall in innodb_cluster.go.ts Replication: Dropped 5.7 from CI (minimal tarball URL broken), test 8.0/8.4/9.5 only.
Fan-in: Added sleep before test_replication for replication catchup.
PostgreSQL multiple: Added sleep after deploy for node startup.
Summary by CodeRabbit
Chores
Refactor
Tests