Skip to content

Antalya 26.8: Antalya-26:6 - iceberg v3: rewriteFiles - Added logic to merge small files into bins - #2737

Merged
zvonand merged 4 commits into
antalya-26.8from
feature/antalya-26.8/pr-2420
Nov 1, 2026
Merged

zvonand merged 4 commits into
antalya-26.8from
feature/antalya-26.8/pr-2420

Conversation

@zvonand

@zvonand zvonand commented Oct 29, 2026

Copy link
Copy Markdown
Member

Changelog category (leave one):

  • New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Implement bin-packingReWriteFiles operation 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:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

Cherry-picked from #2420.

…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
@zvonand zvonand added releasy Created/managed by RelEasy antalya-26.8 Session label (releasy session config) forwardport This is a frontport of code that existed in previous Antalya versions ai-resolved Port conflict auto-resolved by Claude labels Oct 29, 2026
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Workflow [PR], commit [2962c23]

@zvonand zvonand mentioned this pull request Oct 29, 2026
59 tasks done
@zvonand

zvonand commented Oct 30, 2026

Copy link
Copy Markdown
Member Author

@blau-ai

@blau-ai

This comment was marked as outdated.

@zvonand

zvonand commented Oct 30, 2026

Copy link
Copy Markdown
Member Author

@blau-ai fix test_storage_iceberg_no_spark/test_iceberg_history_missing_optional_summary_metrics.py directly in this branch

@blau-ai

blau-ai commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator

Pushed 95eb5f4 to feature/antalya-26.8/pr-2420. It fixes test_storage_iceberg_no_spark/test_iceberg_history_missing_optional_summary_metrics.py.

Why it failed

In Integration tests (arm_binary, distributed plan, 3/4) on 4c9f515, both test cases failed. Both failures come from this PR's new IcebergMetadata::optimize code. The server is behaving as intended; the test still expected the old OPTIMIZE behaviour.

  • [1]: OPTIMIZE now rejects format v1 straight away with OPTIMIZE TABLE is supported only for Iceberg format_version 2, got 1. The test only allowed the old message, Compaction is supported only for format_version 2.
  • [2]: when there are no live position deletes, OPTIMIZE now runs bin-pack compaction. That succeeds and commits a new replace snapshot, so system.iceberg_history returns 3 rows instead of 2. The CI result was ['3', '2', '2']: the two stripped snapshots still read correctly with the missing metrics defaulted to 0, and the extra row is the new snapshot.

What changed in the test

  • The v1 case now allows the new rejection message.
  • The system.iceberg_history check now counts only the snapshot ids of the stripped snapshots, taken from the edited metadata. The extra replace snapshot no longer affects the count, and the test still checks that the stripped metrics default to 0.
  • I updated the comments: the bin-pack path no longer calls getHistory. The system.iceberg_history query is now what actually catches a regression of Iceberg Invalid access: Can not convert empty value ClickHouse/ClickHouse#89037. The Can not convert empty value check on OPTIMIZE is still there.

Only the test changed, no server code. I have not run it locally; CI on this PR needs to confirm the fix.

@zvonand

zvonand commented Oct 31, 2026

Copy link
Copy Markdown
Member Author

@blau-ai

@blau-ai

This comment was marked as outdated.

@zvonand

zvonand commented Oct 31, 2026

Copy link
Copy Markdown
Member Author

@blau-ai

@blau-ai

blau-ai commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator

CI triage for 2962c23de7

Verdict: none of the failures come from the iceberg rewrite / bin-packing code in this PR. The only thing that needs fixing on the PR is the DCO check.

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_iceberg and test_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.

@zvonand
zvonand merged commit fc6d440 into antalya-26.8 Nov 1, 2026
300 of 316 checks passed
@zvonand zvonand added verified Approved for release port-antalya PRs to be ported to all new Antalya releases labels Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-resolved Port conflict auto-resolved by Claude antalya-26.8 Session label (releasy session config) forwardport This is a frontport of code that existed in previous Antalya versions port-antalya PRs to be ported to all new Antalya releases releasy Created/managed by RelEasy verified Approved for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants