Skip to content

[DBI-829] CI e2e testing: Support different MariaDB versions and parameters#4478

Merged
pfcoperez merged 4 commits into
mainfrom
DBI-829/ci/flow/mariadb-cases
Jun 24, 2026
Merged

[DBI-829] CI e2e testing: Support different MariaDB versions and parameters#4478
pfcoperez merged 4 commits into
mainfrom
DBI-829/ci/flow/mariadb-cases

Conversation

@pfcoperez

@pfcoperez pfcoperez commented Jun 23, 2026

Copy link
Copy Markdown
Member
  • Declare different mariadb versions (start with maria-lts) and provide associated image, parameters and env vars as a separate object configuration.
  • Add a dedicated entry for gtid and pos MariaDB entries including changes in Go test code to support GTID in MariaDB tests.

📝 This change requires changes in branch protection conditions.

…de associated image, parameters and env vars as a separate object configuration
…by checking just the version prefix as a discriminator
@github-actions

Copy link
Copy Markdown
Contributor

❌ Test Failure

Analysis: Deterministic MariaDB-compatibility bug: the maria-lts e2e matrix variant fails 100% because the MySQL test setup queries the MySQL-only 'gtid_mode' variable (ERROR 1193: Unknown system variable) and e2eshared.go doesn't recognize the 'maria-lts' version string, failing all TestSwitchboardMySQL subtests and TestMySQLRDSBinlog at 0.00s with no timeout/race/network signatures.
Confidence: 0.96

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

@pfcoperez
pfcoperez marked this pull request as ready for review June 23, 2026 15:38
@pfcoperez
pfcoperez requested a review from a team as a code owner June 23, 2026 15:38
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Comment on lines 37 to +40
{pg: 16, mysql: 'mysql-gtid', mongo: '6.0', ch: 'lts'},
{pg: 17, mysql: 'mysql-pos', mongo: '7.0', ch: 'stable'},
{pg: 18, mysql: 'maria', mongo: '8.0', ch: 'latest'},
{pg: 18, mysql: 'maria-pos', mongo: '8.0', ch: 'latest'},
{pg: 18, mysql: 'maria-gtid', mongo: '8.0', ch: 'stable'},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking to do it the other way around:

          {pg: 16, mysql: '5.7-pos', maria: 11, mongo: '6.0', ch: 'lts'},
          {pg: 17, mysql: '8-gtid', maria: 12, mongo: '7.0', ch: 'stable'},
          {pg: 18, mysql: '9-gtid', maria: 13, mongo: '8.0', ch: 'latest'},

Also we decided to explicitly not support filepos for maria, as maria comes with gtid enabled by default and binlog format is slightly different so the support wouldn't just come for free.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the scope of this PR is just to introduce the abstraction config and maria section wouldn't mess anything up, totally fine to follow the rest of changes in another one

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the idea actually. Proceeding to merge.

@pfcoperez pfcoperez Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking to do it the other way around

I initially worked in that direction (#4476) and there are a couple of reasons that led me to abandon that approach:

  • If we want to keep the change-scope limited, we don't want to address the problem of having to decide in CI script which e2e test need to run in function of the test matrix. That means that given MySQL and MariaDB share the same suites, in this scenario we need to mutually exclude MySQL and MariaDB versions and the matrix looks more like:
{pg: 16, mysql: 'mysql-gtid', mariadb: '', mongo: '6.0', ch: 'lts'},
{pg: 17, mysql: 'mysql-pos', mariadb: '', mongo: '7.0', ch: 'stable'},
{pg: 18, mysql: '', mariadb: 'lts', mongo: '8.0', ch: 'latest'},

This adds noise compared to the approach in this PR.

  • I think we'll eventually replace the flow.yaml scripts with an unified provisioning system like Tilt (Tilt flow: e2e test run using gotestsum outside Tilt #4477, Initial attempt to run Tilt environment in CI #4474). And in this context we have more flexibility to do test selection for example. In this context, I think we should ditch describing the test cases matrix as tied versions for difference sources (postgres, Mongo, mysql, ...), we don't need to test again PG18 just because we want to test another MongoDB version or flavor. This is, the rows of the matrix should shrink not increase. By increasing increasing the number of rows, we increase the amount of duplicated work.

@pfcoperez
pfcoperez merged commit c47b4b1 into main Jun 24, 2026
28 checks passed
@pfcoperez
pfcoperez deleted the DBI-829/ci/flow/mariadb-cases branch June 24, 2026 06:00
@pfcoperez pfcoperez changed the title [CP-829] CI e2e testing: Support different MariaDB versions and parameters [DBI-829] CI e2e testing: Support different MariaDB versions and parameters Jun 24, 2026
pfcoperez added a commit that referenced this pull request Jun 24, 2026
After #4478, the last row in
flow CI definition for the version matrix was not `latest` anymore but
`stable`.

This row is used by the Tilt environment to select the default target
version for ClickHouse. `stable` is not a valid tag hence Tilt
environments broke
pfcoperez added a commit that referenced this pull request Jun 26, 2026
- Expanded unit tests for GTID decoding, including case description
refactor.
- Added e2e roundtrip gtid test.
- Re-just test matrix in flow.yaml (left mariadb-pos in to test the
failure of the newly added tests)

Follows: #4478
Closes: https://linear.app/clickhouse/issue/DBI-829
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants