Skip to content

Antalya 26.8, frontports: Send Iceberg file statistics and task retry delay in the Antalya protocol trailer - #2586

Open
ianton-ru wants to merge 3 commits into
antalya-26.8from
feature/antalya-26.8/read-task-extensions
Open

ianton-ru wants to merge 3 commits into
antalya-26.8from
feature/antalya-26.8/read-task-extensions

Conversation

@ianton-ru

Copy link
Copy Markdown

Frontport of #2482 and #2489 onto antalya-26.8.

Iceberg column statistics and the lock_object_storage_task_distribution_ms wait used to ride inside a JSON command stored in the object path. An upstream worker then tried to open that text as an object key. Both fields now follow the upstream ReadTaskResponse payload, and only when the peer negotiated Antalya protocol version 3. Version 2 on this branch is already the absolute Iceberg delete-file path layout, so the trailer could not reuse version 2 from antalya-26.6. If any worker negotiated a lower version, the lock is ignored for the query and every task path stays an object key. Prefetch does not wait on retry_after_us, and SYSTEM STOP SWARM MODE is rechecked before the next request.

This branch also includes the ExternalStorageCache argument fix for Iceberg DROP PARTITION. antalya-26.8 still fails to link without it. The same fix is already on #2575 and #2578, so those pulls will conflict on these three files when they merge.

Related: #2482
Related: #2489

Changelog category (leave one):

  • Improvement

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

*Cluster reads send Iceberg column statistics and the lock_object_storage_task_distribution_ms wait in the Antalya protocol trailer instead of a JSON command in the object path. Peers that did not negotiate Antalya protocol version 3 are assigned files immediately.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

CI/CD Options

Exclude tests:

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

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • CAS (content-addressed storage; Antalya only)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • OAuth (5m)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

Made with Cursor

ianton-ru and others added 2 commits October 16, 2026 10:43
`getManifestFileEntriesHandle` and `generateManifestList` take the cache after external-path support, but `AlterDropPartitionExecutor` still used the old signatures, so the amd binary build failed.

https: //altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2575&sha=de846f87651f582609e740036d2a9ba9db179950&name_0=PR&name_1=Build%20%28amd_binary%29
https: //github.com//pull/2575
Co-authored-by: Cursor <cursoragent@cursor.com>
…protocol trailer.

Frontport of #2482 and #2489. On `antalya-26.8` version 2 already means absolute Iceberg paths, so `DataFileMetaInfo` and `retry_after_us` are version 3. A peer below that version gets a plain object path, and `lock_object_storage_task_distribution_ms` is ignored for the query.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Workflow [PR], commit [e1e35b8]

@ianton-ru ianton-ru added antalya 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 labels Oct 16, 2026
@ianton-ru ianton-ru changed the title Send Iceberg file statistics and task retry delay in the Antalya protocol trailer Antalya 26.8, frontports: Send Iceberg file statistics and task retry delay in the Antalya protocol trailer Oct 16, 2026
@ianton-ru ianton-ru mentioned this pull request Oct 16, 2026
52 tasks done
@zvonand

zvonand commented Oct 18, 2026

Copy link
Copy Markdown
Member

@blau-ai

1 similar comment
@zvonand

zvonand commented Oct 18, 2026

Copy link
Copy Markdown
Member

@blau-ai

@blau-ai

blau-ai commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

CI triage for 5b5e011

Verdict: 7 red checks — 0 are caused by this PR's code. Six are pre-existing base-branch breakage or infra; one is a non-blocking pytest import collision triggered by the new integration test this PR adds (a test-framework artifact, not a code bug). Every build, Fast test, Stateless, Unit, Stress, AST-fuzzer and BuzzHouse job is green, so the actual change looks healthy.

Check Classification PR-caused?
Regression release oauth (9 scenarios) pre-existing on antalya-26.8 ❌ No
Regression release swarms (3 scenarios) pre-existing on antalya-26.8 ❌ No
Grype Scan keeper (4) + server-alpine (1) base-image CVEs ❌ No
Parser memory check infra (missing master artifact) ❌ No
Integration db disk, old analyzer, **3/8** infra (session timeout, non-blocking) ❌ No
Integration db disk, old analyzer, **4/8** pytest import collision, non-blocking ⚠️ Triggered by PR's new test, but a framework artifact
PR (aggregate) rollup of the above ❌ No

Regression — OAuth & Swarms → pre-existing (not this PR)

Decisive evidence: these same suites are failing on every recent PR into antalya-26.8, including #2592 "Antalya-26.8: Fix build" — a build-only change that cannot affect auth or swarm runtime behaviour. #2592's Swarms run fails on the exact same three scenarios as this PR:

  • /swarms/feature/cluster discovery/check cluster discovery with wrong cluster name
  • /swarms/feature/node failure/cpu overload
  • /swarms/feature/node failure/cpu overload all swarm nodes

The swarm assertions are all assert r.exitcode == exitcode where the query came back with Code: 279 … Cannot connect to any replica for query execution (ALL_CONNECTION_TRIES_FAILED) — i.e. the overloaded swarm replicas were unreachable, the expected flaky symptom of the CPU-overload resilience tests, not a task-distribution logic error. OAuth (9 failures across access control + groups) is unrelated to anything this PR touches and also fails on #2592/#2551.

Cross-check (Regression release swarms / oauth):

Fix: nothing to change in this PR. These are base-branch/infra failures and need to be addressed on antalya-26.8 separately (or re-baselined). They should not block this PR.

Grype Scan (keeper / server-alpine) → pre-existing base-image CVEs

The diff contains only src/, docs/ and tests/integration/** files — no Dockerfiles, no contrib/, no dependency/lockfile changes — so the reported high/critical CVEs live in the base image and system packages, not in anything this PR introduces. Pre-existing; handle via the usual image/dependency bump, not here.

Parser memory check → infra

ERROR: No master clickhouse-examples artifact was found in the "Resolve master binary" step. The job needs the master baseline clickhouse-examples binary to diff against and it wasn't available; the check never got to run the PR binary. Infra, re-run.

Integration tests db disk, old analyzer, 3/8 → infra (non-blocking)

ERROR: session-timeout occurred during test execution after ~2h34m. A whole-shard wall-clock timeout, not a specific test failure, and the node carries do_not_block_pipeline_on_failure: true. Safe to re-run.

Integration tests db disk, old analyzer, 4/8 → the only PR-connected item (non-blocking)

test_iceberg_mixed_antalya_upstream/test.py  ERROR
import file mismatch:
imported module 'test' has this __file__ attribute:
  .../tests/integration/test_export_merge_tree_part_to_iceberg/test.py
which is not the same as the test file we want to collect:
  .../tests/integration/test_iceberg_mixed_antalya_upstream/test.py
HINT: remove __pycache__ / .pyc files and/or use a unique basename for your test file modules

This PR adds tests/integration/test_iceberg_mixed_antalya_upstream/test.py (confirmed new vs antalya-26.8), and it landed in shard 4/8 alongside the pre-existing test_export_merge_tree_part_to_iceberg/test.py. Both directories hold a module literally named test.py with no __init__.py, so when pytest collects them in the same worker it hits a module-basename collision — a well-known ClickHouse integration-runner artifact (usually stale __pycache__ / xdist batching), not a defect in the test's content. The node is do_not_block_pipeline_on_failure: true.

Two things worth noting for you:

  1. Because collection aborted, your new test did not actually run — so this PR's new mixed-Antalya/upstream Iceberg coverage is currently unverified in CI.
  2. Fix: re-run shard 4/8 first (this often clears on a clean worker). If it reproduces, it's a runner pycache/collision issue rather than the test body; the most robust durable fix is giving the new module a unique basename (e.g. test_iceberg_mixed_antalya_upstream.py instead of test.py) so it can never collide — though note the rest of the suite uses bare test.py, so a re-run is the lighter first step.

Bottom line: the PR's own code is green across builds and functional/unit/stress testing. Nothing here requires a code change to the PR. The only item tied to the PR is a non-blocking test-collection collision from the newly-added integration test — re-run shard 4/8 and confirm test_iceberg_mixed_antalya_upstream actually executes and passes. OAuth, Swarms, Grype and Parser-memory are pre-existing base-branch/infra failures (proven by the build-only #2592 showing the identical regression failures).

— automated CI triage; I can't build/run ClickHouse in this environment, so classifications are from the praktika S3 reports and CI logs. Happy to dig deeper into any one of these.

@zvonand

zvonand commented Oct 18, 2026

Copy link
Copy Markdown
Member

@blau-ai "Integration db disk, old analyzer, 4/8 pytest import collision, non-blocking ⚠️ Triggered by PR's new test, but a framework artifact" -- can you fix this directly in this branch?

@blau-ai

blau-ai commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

CI triage for this PR

Verdict: 7 red checks → 0 caused by this PR's product code. 6 are pre-existing failures on antalya-26.8 (they appear identically on sibling PRs that don't touch this code), and 1 is this PR's own new integration test failing to be collected by pytest — a test-harness/caching hiccup, not a code defect, and it's a non-blocking check. Only that last one is worth touching.

Evidence baseline: I compared against two other open PRs into antalya-26.8 — #2575 (GROUP BY over s3Cluster, does not touch the protocol/distributor) and #2578. Both show the same Grype / Parser-memory / oauth / swarms failures as this PR.


1. Integration tests (amd_asan_ubsan, db disk, old analyzer, 4/8) — Failures: 1/905 — ⚠️ PR-related (test harness, non-blocking)

The single failure is this PR's new test failing at collection, not at runtime:

import file mismatch:
imported module 'test' has this __file__ attribute:
  .../tests/integration/test_export_merge_tree_part_to_iceberg/test.py
which is not the same as the test file we want to collect:
  .../tests/integration/test_iceberg_mixed_antalya_upstream/test.py
HINT: remove __pycache__ / .pyc files and/or use a unique basename for your test file modules

This is the classic pytest duplicate-basename collision: both files are named test.py and the integration runner uses the default prepend import mode (no --import-mode=importlib in tests/integration/pytest.ini). The new test_iceberg_mixed_antalya_upstream/test.py collided with the existing test_export_merge_tree_part_to_iceberg/test.py. The test never ran — this is not a product assertion failure, and the check is marked do_not_block_pipeline_on_failure.

Suggested fix (the only change I'd make to the PR):

  • First, just re-run the 4/8 shard. 904/905 sibling test.py files collected fine in the same shard, which strongly suggests a stale __pycache__/.pyc on the reused runner (exactly what the HINT calls out) rather than a deterministic clash.
  • If it recurs, give the new test a unique module filename so it can never collide: rename
    tests/integration/test_iceberg_mixed_antalya_upstream/test.py → .../test_mixed_antalya_upstream.py.
    This is still collected (python_files = test_*/test*.py) and permanently removes the basename clash with test_export_merge_tree_part_to_iceberg/test.py.

2. Integration tests (… 3/8) — ERROR: session-timeout — ❌ not PR-caused (infra/flaky)

Whole-shard watchdog (ran to the 7200s session timeout); no individual test is attributed. The new test lives in shard 4/8, not 3/8, so it can't be the cause. This ASAN shard also timed out/failed on #2578. Safe to re-run.

3. Parser memory check — Failures: 1/2 — ❌ not PR-caused (infra)

Failing step is Resolve master binary: No master clickhouse-examples artifact was found — the baseline master artifact is missing, so there's nothing to compare against. Identical on #2575/#2578. Re-run once the master artifact exists.

4. Grype Scan (keeper: 4 high/critical, server -alpine: 1 high/critical) — ❌ not PR-caused (pre-existing base-image CVEs)

These scan OS/base-image packages in the built Docker images, independent of the C++ diff. Counts are identical on #2575/#2578. Addressing them is a base-image/dependency bump at the branch level, not something this PR can or should do.

5. Regression release oauth — 9 scenario fails — ❌ not PR-caused (auth layer)

Every failure is the same pattern: negative access-control tests expecting HTTP 500 but the server returns 403 with Code: 497 … ACCESS_DENIED. Pure auth / HTTP-status-mapping behavior — no protocol, Iceberg, Remote, connection, or task-distribution involvement. Identical "1 feature failed" on #2575/#2578. Looks like test-expectation drift (500→403) on the branch.

6. Regression release swarms — 3 scenario fails — ❌ not PR-caused (pre-existing/flaky)

These are in this PR's blast radius (object-storage cluster reads), so I checked carefully. Failures: cluster discovery / wrong cluster name (ALL_CONNECTION_TRIES_FAILED, exitcode 33 vs 23) and two node failure / cpu overload (SOCKET_TIMEOUT while executing Remote over object_storage_cluster). PR #2575, which does not touch the protocol or the task distributor, has the byte-identical swarms failures — same cluster-discovery assertion (33 vs 23), same 3 feature fails, same scenario counts. So this is pre-existing on antalya-26.8; the cpu-overload/node-failure cases are timing-sensitive flakes. Not introduced by this PR's retry-delay / trailer change.


PR health

Everything that actually exercises this change is green: all build variants, Fast test, ~13.3k stateless tests, AST fuzzer, unit/gtest suites, and 904/905 of the integration shard. The protocol-version gating in AntalyaProtocol.h (v3 = read-task extensions, v2 stays absolute-path) and the new gtest coverage for ClusterFunctionReadTask look sound in CI. The only PR-attributable red is a pytest collection hiccup on the newly-added integration test — not a code defect.

Net: no PR-caused product failures. Re-run 3/8 and 4/8; if 4/8 still collides, rename the new test.py as above.

I haven't pushed anything. If you'd like, I can deliver that one-line test rename — say whether you want it committed directly to feature/antalya-26.8/read-task-extensions or as a separate blau/* PR, and I'll open it.

Fixes a pytest 'import file mismatch' collection error in the
'Integration tests (..., db disk, old analyzer, 4/8)' shard: without an
__init__.py the new test.py is imported under the bare top-level module
name 'test', which collides with another test.py (e.g.
test_export_merge_tree_part_to_iceberg/test.py) collected in the same
pytest process. Adding an empty __init__.py — the convention followed by
~all other tests/integration/* dirs — makes it import as the
package-qualified module 'test_iceberg_mixed_antalya_upstream.test',
which cannot collide regardless of collection order.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@blau-ai

blau-ai commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

@zvonand Done — committed directly to feature/antalya-26.8/read-task-extensions as requested.

Commit: e1e35b8 — Add __init__.py to test_iceberg_mixed_antalya_upstream integration test

What the failure was

The 4/8 shard reported an ERROR on test_iceberg_mixed_antalya_upstream/test.py (marked do_not_block_pipeline_on_failure, hence non-blocking):

import file mismatch:
imported module 'test' has this __file__ attribute:
  .../tests/integration/test_export_merge_tree_part_to_iceberg/test.py
which is not the same as the test file we want to collect:
  .../tests/integration/test_iceberg_mixed_antalya_upstream/test.py
HINT: ... use a unique basename for your test file modules

Root cause

It's a pytest collection artifact, not a product/test-logic bug. tests/integration/ has no top-level __init__.py, so each test directory is its own package root. The convention here is that every test dir carries an empty __init__.py (~876 of them do) — that makes test.py import under the package-qualified name <dir>.test instead of the bare top-level module test. The new dir was missing it, so when this shard batched it into the same pytest process as another test.py (test_export_merge_tree_part_to_iceberg), the second one to import as bare test hit the mismatch.

Fix

Added an empty tests/integration/test_iceberg_mixed_antalya_upstream/__init__.py. The new test now imports as test_iceberg_mixed_antalya_upstream.test, which cannot collide with the bare test module regardless of collection order — fully eliminating this collision pair.

Note (out of scope, FYI)

test_export_merge_tree_part_to_iceberg/ (an earlier Antalya test, not part of this PR) is also missing its __init__.py. It's now the lone bare-test dir in that pair so it won't collide here, but it could collide with some other bare-test.py in a future batch. Worth adding the same __init__.py to it in a separate change if you want to close that off too — I left it untouched to keep this commit scoped to your PR.

I can't run the integration suite in this environment, so correctness is confirmed by CI on the next run of the 4/8 shard — the import file mismatch should no longer appear.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants