[DBI-829] CI e2e testing: Support different MariaDB versions and parameters#4478
Conversation
…de associated image, parameters and env vars as a separate object configuration
…by checking just the version prefix as a discriminator
❌ Test FailureAnalysis: 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. |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
| {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'}, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Yes, that's the idea actually. Proceeding to merge.
There was a problem hiding this comment.
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.
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
- 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
maria-lts) and provide associated image, parameters and env vars as a separate object configuration.📝 This change requires changes in branch protection conditions.