Antalya 26.6: Backport of #98178 Iceberg compaction using optimize - #2193
Conversation
subkanthi
changed the title
fc76726 to
9626fa1
Compare
subkanthi
changed the title
test_storage_iceberg_with_trino/test.py::test_optimize_manifest_trino_field_ids
|
|
Multiple inserts to create manifests BEFORE: AFTER |
CI triage for #2193Verdict: 7 red checks — 0 caused by this PR's code. 6 are flaky/infra/pre-existing; 1 is the expected consequence of adding a new setting and is fixed in the Head SHA analyzed: Not PR-caused (flaky / infra / pre-existing)1. Stateless tests (amd_debug, parallel) — 3 sub-failures, all cleared by CI's own diagnosis:
2. Stateless tests (amd_debug, distributed plan, s3 storage, parallel)
3. SQLLogic test — 6 new failures vs 10,520 fixed. Every new failure is 4. Regression release settings — 1 of the 2 failing scenarios:
5. Regression release tiered_storage_cas — every failure is 6. Grype Scan (…-alpine) — 1 High: 7. PR — aggregate gate; red only because of the above. PR-related, but no code fix needed hereRegression release settings — the other failing scenario is This is the new setting this PR adds ( Concrete fix (in Suggested next steps
One coverage note (not a failure): all 🤖 automated CI triage · evidence from praktika |
| /*added_data_files=*/added_files, | ||
| /*added_delete_files=*/added_delete_files, | ||
| /*added_position_deletes=*/num_deleted_rows, | ||
| /*added_equality_deletes=*/0); |
There was a problem hiding this comment.
In original PR ClickHouse#98178 this block replaces lines below (sum_with_parent_snapshot(...)), here these lines still there.
There was a problem hiding this comment.
Removed sum_with_parent_snapshot block
subkanthi
added
verified
subkanthi
changed the title
CI triage for
|
| Check | Failing | Classification | PR-caused? |
|---|---|---|---|
| Stateless tests (amd_debug, sequential) | 3 | Infra — test-dataset host unreachable / TOO_SLOW |
❌ No |
| Stateless tests (amd_binary, cas s3 storage, parallel) | 10 | Infra — same host timeout + runner-overload test timeouts | ❌ No |
| Regression release swarms | 14 | Pre-existing — regression suite uses a setting this binary doesn't have | ❌ No |
| Regression release settings | 5 | Snapshot lag; 1 is this PR's new setting, 4 are other branch settings |
1–2. Stateless tests — infrastructure outage (not PR)
Every failing stateless test operates on the shared stateful dataset (test.hits / test.visits), and every failure is a connection or slowness error against the dataset host, not a wrong result:
Code: 1000. DB::Exception: Timeout: connect timed out: 65.108.242.32:6000. (POCO_EXCEPTION)
in query: SELECT sum(cityHash64(*)) FROM test.hits ... # 00167_read_bytes_from_fs
in query: INSERT INTO test.hits_1m SELECT * FROM test.hits ...# 00157_cache_dictionary
Code: 160. DB::Exception: Estimated query execution time (38048s) is too long. Maximum: 900. (TOO_SLOW)
in query: INSERT INTO test.hits_log SELECT ... FROM test.hits # 00077_log_tinylog_stripelog
The CAS-S3 suite shows the same signature: connect timed out: 65.108.242.32:6000 (00020_distinct_order_by_distributed, 00054_merge_tree_partitions), TOO_SLOW (00178_quantile_ddsketch), and Timeout! Killing process group on the heavy TPC-DS queries (04033_tpc_ds_q56/q61/q71/q81/q86/q94, 00152_insert_different_granularity) — a runner that couldn't reach the dataset host and was overloaded.
None of these tests touch Iceberg or anything in this diff. Action: safe to re-run once the dataset host is healthy.
3. Regression release swarms — pre-existing (not PR)
All 14 failures are the same error in one feature (/swarms/feature/fallback to local if empty); the other 11 swarm features pass:
Code: 552. DB::Exception: Unrecognized option '--object_storage_cluster_fallback_to_local_if_empty'. (UNRECOGNIZED_ARGUMENTS)
The clickhouse-regression suite is exercising a setting (object_storage_cluster_fallback_to_local_if_empty) that does not exist in this build (26.6.4.20001.altinityantalya) — i.e. the regression suite is ahead of what this branch ships. This PR neither adds nor removes that setting (its only change to StorageObjectStorageCluster is an additive getCatalog() accessor that delegates to pure_storage), so it cannot be the cause. This will fail identically for any PR on antalya-26.6 until the branch gains that setting or the suite is pinned. Action: pre-existing; track against the branch, not this PR.
4. Regression release settings — snapshot lag (1 expected, 4 pre-existing)
The default values test compares each setting against a recorded snapshot (default_values.py.default values>=26.6_antalya.snapshot). All 5 failures are SnapshotNotFoundError — the setting is simply absent from the snapshot, not a wrong default:
✘ analyzer_compatibility_multiple_joins_qualify_column_names (pre-existing branch setting)
✘ filesystem_cache_wait_for_concurrent_download_timeout_milliseconds (pre-existing)
✘ iceberg_manifest_min_count_to_compact ← added by THIS PR
✘ statistics_max_set_size_for_exact_selectivity_estimation (pre-existing)
✘ throw_on_hive_partitioning_resolution_failure (pre-existing)
iceberg_manifest_min_count_to_compact (UInt64, default 30) is the new setting this PR adds in Settings.cpp / SettingsChangesHistory.cpp, so its snapshot miss is a direct consequence of the PR — but it is expected behaviour, not a defect: this test fails for every newly-added setting until the snapshot is regenerated. The other 4 are unrelated new settings on antalya-26.6 with the same snapshot lag.
Concrete fix (external repo, not this PR): add iceberg_manifest_min_count_to_compact to the settings snapshot in Altinity/clickhouse-regression (settings/tests/snapshots/default_values.py.default values>=26.6_antalya.snapshot) — ideally in the same PR/batch that adds the other four branch settings. No change to this ClickHouse PR is warranted, so I have not pushed anything.
Caveat: the feature's own tests did not run here
Integration and unit tests were all skipped in this workflow config (excluded asan/tsan/msan/arm tags, and the amd integration/unit jobs show SKIPPED). That means this PR's own coverage — tests/integration/test_storage_iceberg_with_spark/test_manifest_compaction.py, test_database_iceberg::test_optimize_manifest_with_catalog, and the gtest_iceberg_* unit tests — was not exercised by this run. The red checks are all unrelated to the compaction code, but the compaction code isn't validated by them either. Consider triggering an integration-test run (label/re-run with the iceberg integration jobs enabled) before merge.
Health check
The changed C++ is contained (Iceberg metadata/compaction, an additive OPTIMIZE … MANIFEST-style parser path, one new UInt64 setting, and an additive cluster accessor) and builds cleanly — every Build job is green, as are Fast test, Stateless amd_debug parallel, s3 storage, cas storage, Stress, AST/BuzzHouse fuzzers, SQLLogic and SQLStorm. No failure in this run implicates the PR's logic.
(Analysis from CI artifacts only — I can't build or run ClickHouse in this environment; correctness is ultimately confirmed by a clean CI run.)
|
@subkanthi I know it's a backport but still can you have a look if these make sense for this PR. PR #2193 audit findingsIceberg manifest compaction via High: Commit-failure cleanup can delete manifests after the new snapshot is already visibleAfter the compacted metadata is published, a later catalog failure (or an exception in the same This happens in two commit layouts:
Medium: Manifest-list partition bounds are dropped for common Iceberg partition typesThe rewrite recomputes each compacted manifest-list entry’s ClickHouse’s partition-transform result types hit the rejected set on ordinary tables:
After compaction those summaries keep null bounds. Spark (and any reader that prunes from manifest-list partition stats) cannot prune those partitions and must open extra manifests. Row data is unchanged, but the rewritten metadata no longer matches the source manifests. A narrower extra failure: The bucket-partition integration test only round-trips rows through Spark; it does not assert non-null Low: Rewritten data manifests drop optional per-file fields the parser never readsManifest-only rewrite copies Those fields are not loaded by |
Selfeer
added
verified
6 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Add support for iceberg compaction of manifest files using optimize command.
Documentation entry for user-facing changes
Adds OPTIMIZE TABLE ... MANIFEST to trigger manifest-only compaction for Iceberg tables, backport of ClickHouse#98178
This PR covers rewriting manifest files. The default value is 30, but can be overwritten using
iceberg_manifest_min_count_to_compact. The setting is the threshold: compaction runs only if the current snapshot's manifest list contains strictly more than this many manifest files. Setting this value to 0 means "always compact"CI/CD Options
Exclude tests:
Regression jobs to run: