Make ci_finished_builds engine swap work on ClickHouse 23.x

Narendranrequested to merge
narendran-kannan/593129/ch-23-x-compat into master

What does this MR do and why?

Four follow-up fixes to !240219 (merged) and !240627 (merged) to make 20260108073937_alter_ci_finished_builds_engine_with_version and its spec safe across every supported ClickHouse version.

Why this matters on master

Per GitLab docs, GitLab 19.0+ requires ClickHouse 25.x or 26.x. Master's CI matrix only exercises CH 25 and 26, so the CH-23.x/24.x-specific failures don't surface on master pipelines.

But the same migration file ships on 18-10-stable-ee and 18-11-stable-ee where CH 23.x and 24.x are both supported and tested, and on customer self-managed deployments running anything from CH 23.x through 26.x. Keeping master and the stable branches identical (the "strict subset" rule for backports) means master needs every fix the backports need.

Backports to 19.0 (!240591 (merged)), 18.11 (!240592 (merged)), and 18.10 (!240593 (closed)) cherry-pick this commit.

Changes

1. drop_tmp_table no longer raises on ClickHouse 23.x

The current code is:

def drop_tmp_table
  execute 'DROP TABLE IF EXISTS ci_finished_builds_tmp SETTINGS max_table_size_to_drop = 0'
end

max_table_size_to_drop = 0 is only valid as a query-level setting on ClickHouse 24.0+. On CH 23.x the same setting exists but is server-level only, so using it inline raises:

Code: 115. DB::Exception: Setting max_table_size_to_drop is neither a builtin
setting nor started with the prefix 'SQL_' registered for user-defined settings.
(UNKNOWN_SETTING) (version 23.11.3.23 (official build))

Worse: simply omitting the SETTINGS clause is unsafe because the default server-side limit is 50 GB. Any customer whose ci_finished_builds_tmp exceeds 50 GB — realistic on GitLab Dedicated — would fail the DROP and end up with an orphan table. There is no query-level / session / HTTP header way to override the limit on CH 23.x; the only escape hatches are server config changes or filesystem flag files, neither available from inside a Rails migration.

Fix: on CH 23.x, ALTER TABLE ci_finished_builds_tmp DETACH PARTITION ID '...' for every partition before DROP TABLE. DETACH PARTITION is not subject to the size check (verified empirically with a forced 1-byte limit); the table then has 0 active bytes and DROP TABLE always succeeds. The detached parts directory is removed along with the table directory, so no disk leak. CH 24+ keeps the cleaner one-shot DROP with the SETTINGS override.

2. Spec no longer hardcodes a CH-version-specific SETTINGS string

The spec previously had:

let(:expected_settings_clause) do
  "index_granularity = 8192, use_async_block_ids_cache = true, " \
    "deduplicate_merge_projection_mode = 'rebuild'"
end

it 'keeps the SETTINGS clause identical on the swapped table' do
  expect { migration.up }
    .not_to change { current_settings_clause }
    .from(expected_settings_clause)
end

This is brittle. ClickHouse 23.x reports use_async_block_ids_cache = 1 (booleans serialize as 0/1 there, not true/false), and it has no deduplicate_merge_projection_mode setting at all. The hardcoded string fails on CH 23.x even when the actual behaviour is correct.

Replace the hardcoded from(expected_settings_clause) with a plain not_to change { current_settings_clause }. This still catches the regression we care about (the swap accidentally drops or adds engine-level settings) but no longer depends on CH-version-specific formatting.

3. supports_deduplicate_merge_projection_mode? gate threshold corrected

The gate previously claimed CH 24.1+ supports the setting:

(major == 24 && minor >= 1) || major >= 25

Empirical docker testing shows that deduplicate_merge_projection_mode actually first appears in CH 24.8 — CH 24.1, 24.2, 24.5, 24.6 and 24.7 all raise UNKNOWN_SETTING if it is set at storage level. Fix the threshold to minor >= 8. This is a pre-existing bug carried over from !240219 (merged); not triggered by any CI matrix (CI uses CH 23.11.3, 24.9.1, 25.x, 26.x) but would break the migration for self-managed customers running CH 24.0–24.7.

4. create_tmp_table is now self-healing across interrupted migration runs

Previously the migration used CREATE TABLE IF NOT EXISTS ci_finished_builds_tmp .... If a prior run crashed between steps (OOM, pod restart, EXCHANGE failure) or if an operator left a stale ci_finished_builds_tmp behind, the IF NOT EXISTS would silently skip the create. The subsequent ATTACH PARTITION could then either:

  • fail loudly with INVALID_PARTITION_VALUE / INCOMPATIBLE_COLUMNS if the stale table has a different schema; or
  • worse, succeed but onto a tmp table that already has partial data, producing duplicate parts on retry.

Fix: create_tmp_table now calls drop_tmp_table first and uses a plain CREATE TABLE (no IF NOT EXISTS). Since the tmp table only ever holds data that is also still in the source table (ATTACH PARTITION FROM copies, not moves — verified empirically), dropping a leftover tmp is always safe. The plain CREATE then loudly fails on any unexpected race instead of silently corrupting state.

Verified with a failure-and-retry test matrix simulating crashes at each of the 4 migration steps × 5 ClickHouse versions = 50 sub-tests, all passing: row count preserved, engine swap durable, extra columns and projections survive every interruption point.

Implementation notes

The shared clickhouse_version_major_minor helper is memoized so up issues a single SELECT version() round-trip, and defaults the minor component to 0 to guard against a non-dotted version string (defensive — addresses Duo review feedback on the previous push).

Database changes

ClickHouse only. The post-deploy migration is amended in place; no new migration files. No PostgreSQL changes. No behavioural change on modern ClickHouse (24.8+) — the SETTINGS max_table_size_to_drop = 0 clause is still emitted there.

Local verification

End-to-end migration sequence executed against five ClickHouse versions via docker:

ClickHouse drop_tmp_table CREATE … AS source ATTACH PARTITION EXCHANGE TABLES Spec passes Failure-retry × 10 scenarios
23.11.3 OK ✓ (DETACH+DROP) OK ✓ OK ✓ OK ✓ should pass (CI will confirm) 10/10 ✓
24.1.8 OK ✓ OK ✓ (no dedup setting) OK ✓ OK ✓ not in CI 10/10 ✓
24.9.1 OK ✓ OK ✓ OK ✓ OK ✓ should pass 10/10 ✓
25.12.3 OK ✓ OK ✓ OK ✓ OK ✓ passes (verified) 10/10 ✓
26.2.13 OK ✓ OK ✓ OK ✓ OK ✓ should pass 10/10 ✓

Failure-retry scenarios cover: crash after CREATE; mid-ATTACH; after-all-ATTACH-before-EXCHANGE; after-EXCHANGE-before-DROP; idempotency (full re-run); stale tmp with wrong schema; stale tmp with matching schema + partial data; idempotency under forced 1-byte size limit; extra columns + projection preserved across mid-attach failure.

References

How to set up and validate locally

To reproduce the CH 23.x drop_tmp_table size-limit failure and the fix:

docker run -d --name ch2311 -p 18231:8123 clickhouse/clickhouse-server:23.11.3.23-alpine
sleep 5
# Force a tiny server-side limit to simulate a >50 GB production table
docker exec ch2311 sh -c "echo '<clickhouse><max_table_size_to_drop>1</max_table_size_to_drop></clickhouse>' > /etc/clickhouse-server/config.d/test.xml"
docker restart ch2311
sleep 5

ch() { curl -s --data-binary "$1" "http://localhost:18231/"; }
ch "CREATE TABLE x (id Int64) ENGINE = MergeTree PARTITION BY (id % 2) ORDER BY id"
ch "INSERT INTO x VALUES (1), (2)"

# OLD form — fails (size > limit, and SETTINGS override is unknown on CH 23.x):
echo "OLD:"
ch "DROP TABLE x SETTINGS max_table_size_to_drop = 0"

# NEW form — DETACH each partition, then DROP:
echo "NEW:"
for p in 0 1; do ch "ALTER TABLE x DETACH PARTITION ID '$p'"; done
ch "DROP TABLE x"
echo "OK"

docker rm -f ch2311

Expected: OLD prints Code: 115 UNKNOWN_SETTING, NEW prints nothing then OK.

To verify the spec on modern CH:

bundle exec rspec spec/db/click_house/post_migrate/20260108073937_alter_ci_finished_builds_engine_with_version_spec.rb

Expected: 8 examples, 0 failures.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Narendran

Merge request reports