Repository navigation
Antalya 26.8: Antalya-26:6 - iceberg v3: rewriteFiles - Added logic to merge small files into bins - #2737
Conversation
…next commit) --- Original cherry-pick message follows: Merge pull request #2420 from Altinity/iceberg_rewrite_files_compaction Antalya-26:6 - iceberg v3: rewriteFiles - Added logic to merge small files into bins # Conflicts: # src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp # src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp # src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp # src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h # src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp # src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.h
Kept base `overwriteIsPositionDeleteOnly` check in `tryGetAppendUpdate`. Kept the `CLICKHOUSE_CLOUD` branch of `IcebergMetadata::optimize`, with the PR's bin-pack logic in the `#else` branch. Adapted: `IcebergMetadata::optimize` uses `external_storages` (renamed from `secondary_storages` on antalya-26.8) Adapted: `ManifestListEntryCounts` keeps base `added_snapshot_id`/`added_sequence_number`; base `counts_are_added` callers in `Compaction.cpp` translated to added_*/existing_* counts Adapted: `tryWriteMetadataFileAndVersionHint` takes base `version_hint_names_metadata_file` out-param and keeps base hint-retry warning and `UNSUPPORTED_METHOD` rethrow
This comment was marked as outdated.
This comment was marked as outdated.
|
@blau-ai fix |
|
Pushed 95eb5f4 to Why it failedIn
What changed in the test
Only the test changed, no server code. I have not run it locally; CI on this PR needs to confirm the fix. |
This comment was marked as outdated.
This comment was marked as outdated.
CI triage for
|
| Check | Failing tests | Classification |
|---|---|---|
| DCO | — | PR housekeeping: none of the 4 commits have Signed-off-by |
| Integration (arm_binary, distributed plan, 1/4) | 21× test_export_partition_iceberg.py::* |
Pre-existing |
| Integration (arm_binary, distributed plan, 2/4) | 3× test_database_backup/test.py::* |
Pre-existing |
| Integration (arm_binary, distributed plan, 3/4) | 2× test_custom_s3_disk_remote_url_allow_hosts.py::* |
Pre-existing |
| ″ | test_cas_early_commit_restart::test_crash_between_early_commit_and_rename_leaves_no_part |
Not PR-related (CAS-only; likely flaky) |
| Stress test (arm_release) | Cannot start clickhouse-server |
Infra / startup timeout |
| Regression aarch64 s3_export_part | 19 failed, 1 errored | Infra cascade (MinIO) |
| Regression release + aarch64 swarms | 3 scenarios | Pre-existing |
The iceberg suites relevant to this PR pass: Regression iceberg_1 / iceberg_2 on both release and aarch64, plus s3_export_partition. The new test_bin_pack_rewrite.py and gtest_bin_pack_rewrite don't show up among the failures.
Note: the arm jobs ran here but were skipped on the sibling PRs I compared against (excluded tag 'arm'). So for the arm-only failures, the "pre-existing" evidence comes from the same tests failing on amd jobs of other antalya-26.8 PRs.
1. DCO — needs action
All commits lack Signed-off-by: efc9d2cf1e, 4c9f515c29, 95eb5f4073 (authored by blau-ai) and 2962c23de7. Fixing it means rewriting history: git rebase --signoff antalya-26.8 plus a force-push. I'm not allowed to force-push, so the author has to do it. The alternative is to accept DCO as non-blocking, which is how it's being treated on #2750, where DCO also fails.
2. test_storage_iceberg_with_spark/test_export_partition_iceberg.py (21 tests): pre-existing
Code: 497. DB::Exception: default: Not enough privileges. To execute this query, it's necessary to have the grant SHOW NAMED COLLECTIONS SECRETS ON s3.
at attach_ch_iceberg -> helpers/iceberg_utils.py:480 create_iceberg_table
All 21 fail during setup, at CREATE TABLE ... (s3 named collection), before any export or rewrite code runs. The same error appears 84 times in Integration tests (amd_asan_ubsan, db disk, old analyzer, 5/8) across #2735, #2750, #2729 and #2715, none of which touch iceberg writes. The problem is the test's access setup (the default user lacks SHOW NAMED COLLECTIONS SECRETS) on antalya-26.8, not this PR.
3. test_database_backup/test.py (3 tests): pre-existing
Code: 49. DB::Exception: Chosen multipart upload for an empty file. This must not happen. (LOGICAL_ERROR)
BackupsWorker: Failed to make backup Disk('backup_disk_s3_plain_rustfs', 'test_table_backup')
This is a backup to an s3_plain (rustfs) disk under a CAS policy, with no iceberg code involved. The same test fails heavily on the siblings: 68× in amd_asan_ubsan 6/8 and 16× in the targeted job.
4. test_backward_compatibility/test_custom_s3_disk_remote_url_allow_hosts.py (2 tests): pre-existing
Timed out while waiting for instance 'node' ... to start during cluster.start(). It also fails on the siblings: 8× in amd_asan_ubsan 7/8 and 2× in the targeted job.
5. test_cas_early_commit_restart (1 test): not PR-related
_assert_fsck_clean: assert '3' == '0' (SYSTEM CAS FSCK found 3 leftover objects)
The test kills the server while a MergeTree INSERT is parked on a CAS disk, then runs a CAS fsck. This PR doesn't touch CAS or MergeTree commit paths. I found no other failure of this test, so I'd call it a probable flake in the GC/fsck timing. It's worth a re-run; open an issue if it repeats.
6. Stress test (arm_release): infra / slow startup
The server never started listening on :9000 within the 120 s window during setup, before any stress queries ran. The server log shows a 40 s stall while loading system tables, with ThreadFuzzer enabled:
system.query_metric_log: Loaded data parts (7 items) took 41.673 seconds
The log ends mid-load (still loading system.transactions_info_log parts at 14:39:53), and fatal.log is empty: no crash, no exception. No iceberg tables are involved at that point. Safe to re-run.
7. Regression aarch64 s3_export_part: infra cascade
The first failure is export part/network/minio interruption → MinIO health check failed. Then clickhouse interruption and concurrent alter hit ClickHouse server is not healthy, and the rest of the suite fails or errors with MaxRetryError / AssertionError (most in ~160 ms each). The suite tests MergeTree part export and passes on release x86 for this same commit (RegressionTestsRelease / S3Export (part)). Safe to re-run.
8. Regression swarms (release + aarch64): pre-existing
The same 3 scenarios fail on #2735, #2750 and #2729:
cluster discovery/check cluster discovery with wrong cluster name, node failure/cpu overload, node failure/cpu overload all swarm nodes.
Suggested next steps
- DCO: sign off the commits and force-push (author only), or accept DCO as non-blocking.
- Stress (arm_release) and Regression aarch64 s3_export_part: re-run these.
test_export_partition_icebergandtest_database_backup: these are base-branch problems that fail on every antalya-26.8 PR I checked. Track them separately; there's nothing to change in this PR.
Evidence: result_pr.json, compared with the reports for #2735, #2750, #2729 and #2715.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Implement bin-packing
ReWriteFilesoperation that Identifies small data files below a target size threshold(defined in Settings), groups them and merges to a large file, creates a snapshot that marks the old ones as DELETED and the new larger merge file as ADDED.Leave all other files, manifests, and history untouched (#2420 by @subkanthi).
CI/CD Options
Exclude tests:
Regression jobs to run:
Cherry-picked from #2420.