Skip to content

Enable cas s3 stress - #2292

Open
alsugiliazova wants to merge 4 commits into
antalya-26.6from
enable-cas-s3-stress
Open

Enable cas s3 stress#2292
alsugiliazova wants to merge 4 commits into
antalya-26.6from
enable-cas-s3-stress

Conversation

@alsugiliazova

Copy link
Copy Markdown
Member

Stress already launches stateless tests at random in parallel (clickhouse-test --stress-tests, several clients, no result validation). Those jobs never installed a CAS default disk, so CAS was only covered by the dedicated stateless lanes.

This adds one CAS-S3 stress job per AMD sanitizer build, plus the non-sanitizer debug package:

  • Stress test (amd_debug, cas s3 storage)
  • Stress test (amd_asan_ubsan, cas s3 storage)
  • Stress test (amd_tsan, cas s3 storage)
  • Stress test (amd_msan, cas s3 storage)

There is no separate amd_ubsan stress package; UBSan is already in amd_asan_ubsan.

Wiring is shared across those jobs:

  • Job name sets USE_CAS_S3_STORAGE_FOR_MERGE_TREE=1. The "cas s3" check must run before the plain "s3" substring check, otherwise install.sh would still pick the S3 default policy.
  • stress_runner.sh starts RustFS (MinIO OSS cannot enforce If-Match deletes) and creates the hits/visits tables on cas_s3.
  • clickhouse-test gets --cas-s3-storage so no-cas-storage / no-object-storage / no-s3-storage tests are skipped.
  • Post-stress system-log dump goes through clickhouse-client while the server still owns the CAS root (clickhouse-local starts with uuid 0 and refuses the claim).

First runs may surface real CAS bugs under thread-fuzzer / fault injection; treat those as product signals, not just CI wiring.

Changelog category (leave one):

  • CI Fix or Improvement (changelog entry is not required)

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

Not required.

Alsu and others added 2 commits August 31, 2026 11:47
Stress already runs stateless tests at random in parallel; this lane
installs cas_s3 as the default MergeTree policy (RustFS, not MinIO)
so CAS is actually on that path. "cas s3" must win over the plain
"s3" substring or install.sh would still pick the S3 default.

Co-authored-by: Cursor <cursoragent@cursor.com>
Match the AMD stress sanitizer set plus the non-sanitizer debug
package, not only ASan.

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

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown

Workflow [PR], commit [894658a]

The stress job uploads only files found under `/test_output` and the server
log directory, but `start_rustfs` writes to `ci/tmp/rustfs.log`. RustFS is
the storage backend for the `cas_s3` policy, so its log is the only record
of pool-side failures such as `If-Match` mismatches or request saturation.
Copy it into `/test_output` compressed so it reaches the CI report.

Co-authored-by: Cursor <cursoragent@cursor.com>
@blau-ai blau-ai mentioned this pull request Sep 2, 2026
30 tasks
@CarlosFelipeOR
CarlosFelipeOR self-requested a review September 4, 2026 11:57
@CarlosFelipeOR CarlosFelipeOR removed their assignment Sep 4, 2026
@CarlosFelipeOR CarlosFelipeOR added antalya antalya-26.6 cicd Improvements and fixes to the CICD process labels Sep 4, 2026
@CarlosFelipeOR

CarlosFelipeOR commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Reviewed the CI wiring against antalya-26.6 and run 33390839603 (attempts 1 and 2), with static analysis assisted by Claude Opus 5.

Wiring checks out: "cas s3" before "s3", env passthrough into install.sh, start_rustfs before start_server, TEMP_POLICY/port 11121 matching the policy config, no-cas-storage gating, upgrade_runner.sh compatibility, the skipped default-volume block at stress_runner.sh:244, and the regenerated YAMLs (master.py/pull_request.py are the only enabled workflows consuming stress_test_jobs).

None of the three findings below blocks the PR. #1 is the one worth doing: it keeps a single dump path across all stress lanes and reuses the technique the repo already has for this exact problem. #2 loses the RustFS log every time a CAS job dies before the end of the script; #3 is cosmetic.

1. <readonly>true</readonly> instead of moving the dump. clickhouse_proc.py:1289 already solves the CAS server-root claim by injecting <readonly>true</readonly> next to <metadata_type>cas</metadata_type> — no mount-slot claim, which is all a dump needs, and it's the path the stateless CAS lanes use. It also keeps one dump path for every stress lane instead of forking it for CAS only. Reusing it keeps clickhouse-local and avoids two side effects:

  • The dump pipeline has no pipefail, so its exit status is zstd's — a client-side failure yields a silently truncated .tsv.zst. It also skips LOGS_SAVER_CLIENT_OPTIONS (clickhouse_proc.py:48), which the existing clickhouse-local path applies. Not academic: the dump takes 39s/31s/64s per table on the tsan rerun, and the server hits MEMORY_LIMIT_EXCEEDED under this load.
  • Dumping before stop_server puts its server-side output in final.log, which check_logs_for_critical_errors (stress_tests.lib:337) greps for Code: 499 The specified key does not exist — reachable via CAS reads with cas_gc_interval_sec=5. Hasn't fired yet.

SYSTEM FLUSH LOGS missing either way.

2. rustfs.log is collected only on the success path. The copy is the last statement of stress_runner.sh, past every || exit 1. Stress test (amd_asan_ubsan, cas s3 storage) died at the post-stress start_server on the latest attempt and never reached it, so its report carries no rustfs.log at all — only job.log, test_results.tsv, application_errors.txt, fatal.log, the server logs and stderr/stdout. The one artifact that records what the storage backend did is absent from exactly the runs where you need it.

Collecting it outside the container — in stress_job.py, right after Utils.fix_ownership_after_docker at :309, copying ci/tmp/rustfs.log into result_path — runs whatever the container's exit code, survives a SIGKILL/timeout where a bash trap would not, and picks up minio.log/azurite.log/kafka.log too. process_results already uploads everything in result_path.

3. Dead branch. USE_CAS_STORAGE_FOR_MERGE_TREE is handled in stress.py:135 and both new stress_runner.sh guards, but nothing can set it — stress_job.py only matches "cas s3" — and there is no TEMP_POLICY branch for it, so a future local-CAS job would create its tables on a different policy than the server default. Either drop the three mentions, or complete it: elif "cas storage" in check_name: in stress_job.py plus elif [[ "$USE_CAS_STORAGE_FOR_MERGE_TREE" == "1" ]]; then TEMP_POLICY="cas" in stress_runner.sh.

@alsugiliazova

Copy link
Copy Markdown
Member Author

Re-reviewed the wiring against 6e29e68 and 33390839603 (attempts 1 and 2). Agree with the review: none of the three findings should block this PR, #2 is the one that actually loses evidence on the failures these lanes exist to catch, and #3 is dead code.

#1 — agree on the design, two nits on the evidence.

<readonly>true</readonly> next to <metadata_type>cas</metadata_type> is the right dump path. clickhouse_proc.py already does this for the stateless CAS scrape, and it keeps a single clickhouse-local dump after stop_server for every stress lane. The client-before-stop fork is a workaround, not a second dump strategy we want to keep.

Dump timings on the tsan rerun match: query_log 39s, trace_log 31s, metric_log 64s (20:10:0820:12:22 in that job.log). Dumping through the live server also writes that SELECT into final.log, which check_logs_for_critical_errors greps for Code: 499 The specified key does not exist — real risk with cas_gc_interval_sec=5, even if it has not fired yet. SYSTEM FLUSH LOGS is missing on both paths.

Nits:

  • LOGS_SAVER_CLIENT_OPTIONS (--max_memory_usage 10G …) is applied by the stateless scrape in clickhouse_proc.py, not by stress's existing collect_query_and_trace_logs clickhouse-local path. Injecting <readonly>true</readonly> un-forks the CAS lane; picking up those client options would be a separate change to the shared stress dump, which no stress lane has today.
  • MEMORY_LIMIT_EXCEEDED on that tsan rerun is not the dump. err.log has fault-injected tracker failures plus 01675_data_type_coroutine GenerateRandom hits at 1 GiB, all during stress (21:53 / 21:56). Zero MEMORY_LIMIT_EXCEEDED in the dump window. The live-server dump is still the worse design; it just did not OOM this run.

Will switch the dump to the readonly clickhouse-local path in a follow-up.

#2 — agree, and this is the one to do.

amd_asan_ubsan, cas s3 storage attempt 2 died at the start_server after configure (the restart before stress.py, not the post-stress start_server 10) and exit 1'd. The job's own upload list is job.log, test_results.tsv, application_errors.txt, fatal.log, server logs, stderr/stdout — no rustfs.log. The S3 prefix still has rustfs.log.zst from attempt 1; listing the prefix after a rerun is not the same as what this attempt uploaded.

The copy is the last statement of stress_runner.sh, past every || exit 1. Host-side copy in stress_job.py after Utils.fix_ownership_after_docker is the right place: /repo is the checkout bind-mount, ClickHouseProc.RUSTFS_LOG is ci/tmp/rustfs.log, and that file survives container SIGKILL where a bash trap would not. process_results already uploads everything in result_path.

#3 — agree it is dead; drop it rather than complete it.

USE_CAS_STORAGE_FOR_MERGE_TREE is read in stress.py:135 and both new stress_runner.sh dump guards, but stress_job.py only matches "cas s3", and there is no TEMP_POLICY="cas" branch. Completing a local-CAS stress job is out of scope here. Drop the three mentions so a future local-CAS lane cannot create hits/visits on a different policy than the server default.

Wiring checks ("cas s3" before "s3", env passthrough, start_rustfs before start_server, port 11121, no-cas-storage gating, skipped default-volume block) look correct to me as well.

Opening the disk read-only skips the server-root claim that made
`clickhouse-local` unusable, so every stress lane keeps one post-shutdown dump
instead of forking to `clickhouse-client`. Flush the logs before the final stop,
collect the helper logs host-side in `stress_job.py` so an early abort still
publishes rustfs.log, and drop the unreachable `USE_CAS_STORAGE_FOR_MERGE_TREE`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: CarlosFelipeOR <carlosfelipeor@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya antalya-26.6 cicd Improvements and fixes to the CICD process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants