Skip to content

Antalya 25.8 Backport of #95203: S3Queue auxiliary Zookeeper support#2115

Open
mkmkme wants to merge 1 commit into
antalya-25.8from
backports/antalya-25.8/95203
Open

Antalya 25.8 Backport of #95203: S3Queue auxiliary Zookeeper support#2115
mkmkme wants to merge 1 commit into
antalya-25.8from
backports/antalya-25.8/95203

Conversation

@mkmkme

@mkmkme mkmkme commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Same as #1357, but for Antalya 25.8

Changelog category (leave one):

  • Improvement

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

S3Queue auxiliary Zookeeper support using keeper_path setting from s3Queue (ClickHouse#95203 by @lesandie)

25.8.15 Backport of ClickHouse#95203: S3Queue auxiliary Zookeeper support
@mkmkme mkmkme added antalya backport Backport antalya-25.8 25.8 25.8 Altinity Stable labels Jul 25, 2026
@github-actions

Copy link
Copy Markdown

Workflow [PR], commit [19aa4bd]

@mkmkme

mkmkme commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator Author

@blau-ai

@blau-ai

blau-ai commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

CI triage — @mkmkme

Verdict: No failure is a genuine regression from this PR. The C++ change is a behaviour‑preserving refactor (getZooKeeper()ObjectStorageQueueMetadata::getZooKeeper(log, zookeeper_name)context->getDefaultOrAuxiliaryZooKeeper(name)), which for the default ZooKeeper path is identical to before. All red checks are either infra/timeout, or pre‑existing/known‑flaky suites that a change to ObjectStorageQueue cannot plausibly affect (LDAP, RBAC, SSL, session_timezone, swarms, TieredStorage). Note CI is still running on this SHA, so the list may shift.

Rough tally: 0 PR‑caused · ~1 needs‑a‑look (pre‑existing flaky test) · rest infra/flaky/pre‑existing.


Integration tests (amd_asan, flaky check)fail (passed: 95, fail: 10)

  • Decisive log: the whole fail: 10 is one test — test_streaming_to_many_views failing 10/10 iterations (ordered-1..5, unordered-1..5). test_failed_retry (the other 95) passed every iteration.
  • The failure is a 120 s timeout waiting for a server‑log line:
    Exception: Command [... grep -Em 1 'File streaming_to_many_views_ordered_data/b_39.csv failed at try 2/2, retries node exists: true'] return non-zero code 1
  • Classification: pre‑existing / known‑flaky, not PR‑caused.
    • test_streaming_to_many_views is not modified by this PR (the PR only adds test_auxiliary_zookeeper_keeper_path / test_auxiliary_zookeeper_missing_configuration), and it is documented‑flaky upstream — ClickHouse/ClickHouse#75573 (label flaky test).
    • The log message it greps for (ObjectStorageQueueIFileMetadata.cpp:507, "File {} failed at try {}/{}, retries node exists: {} ...") is unchanged by the diff — so it's not a format mismatch, it's the retry not completing within the 120 s window under the flaky‑check's 5×‑parallel ASan load.
    • The original PR 25.8.15 Backport of #95203: S3Queue auxiliary Zookeeper support #1357 never ran this check (it was skipping there); it only fired here because this PR touches test_0.py, which makes the harness re‑run tests from that file. It happened to select test_streaming_to_many_views/test_failed_retrynot the new aux‑ZK tests.
  • Suggested next step: no code fix belongs in this backport. Since the failing test is pre‑existing flaky and unrelated to the feature, treat it as a known‑flaky waiver. A re‑run is unlikely to help (10/10 fail under this harness); hardening that test (bump the 120 s wait) is a separate change, not this PR.

Build (amd_release)fail, exit code [-15]

  • Decisive log: WARNING: Timeout exceeded [7200], send SIGTERM ... ERROR: Run failed with exit code [-15]. The build reached [15382/15531] objects, then stalled and hit the 2‑hour wall‑clock cap.
  • Classification: infra / build timeout, not PR‑caused. The code compiles fine — the amd_asan build on this same SHA succeeded and its binary was consumed by the integration jobs.
  • Suggested next step: safe to re‑run the build job.

Stress test (amd_debug), Stress test (amd_tsan)fail, Killed by signal (in clickhouse-server.log)

  • Classification: infra/flaky. Stress tests don't specifically exercise ObjectStorageQueue; "killed by signal" is a common stress‑harness/OOM pattern. Not linked to this diff.
  • Suggested next step: re‑run; compare against base if it persists.

Regression: session_timezone (x86 + aarch64), swarms (x86 + Release), and the aarch64 suites — LDAP, RBAC (2)/(3), S3 (azure, 2), SSLServer (2), TieredStorage (local/s3amazon/s3gcs)

  • Classification: pre‑existing / flaky / infra, not PR‑caused. A change confined to src/Storages/ObjectStorageQueue/ cannot break LDAP auth, RBAC, SSL, session timezone handling, swarms, or tiered storage. session_timezone fails one scenario on both arches (a hallmark of an environment/pre‑existing issue), and the base branch antalya-25.8 itself currently has red/cancelled CI on recent merges.
  • Suggested next step: waive as pre‑existing; if a gate requires green, re‑run the specific suites, but they are not this PR's responsibility.

On the change itself (health check): the diff cleanly threads a zookeeper_name through the file‑metadata/ordered‑metadata call chain and centralises ZK acquisition in ObjectStorageQueueMetadata::getZooKeeper. One backport‑specific note worth a glance from a reviewer: getZooKeeper(LoggerPtr /* log */, name) carries an unused log param with the comment "Keep the log parameter to match upstream signature; not used in this build" — harmless, just a divergence from the head‑branch version. The correctness of the auxiliary‑ZooKeeper path is exercised by the two new integration tests; those will be validated once a full (non‑flaky‑check) integration run is green.

Analysis is CI‑evidence‑based; I can't build/run ClickHouse in this container. I did not push any changes — the one check touching PR‑adjacent code is a pre‑existing flaky test, so there's nothing to fix in the diff. Happy to open a separate PR hardening test_streaming_to_many_views if you want it.

@mkmkme mkmkme added the verified Approved for release label Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

25.8 25.8 Altinity Stable antalya antalya-25.8 backport Backport verified Approved for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants