All committers have signed the CLA.

@rschu1ze thanks for taking a look! 😄

Workflow [PR], commit [d2720eb]

Summary: ❌

AI Review

Summary

This PR adds an optional constant precision argument to formatReadableSize, formatReadableDecimalSize, and formatReadableQuantity, preserves the default behavior when omitted, validates argument shape/type, and adds focused stateless coverage for normal usage and boundary/error cases. I did not find unresolved correctness, safety, or contract issues in the current diff.

Final Verdict

Status: ✅ Approve

@groeneai Make yourself useful (and Nvidia stocks go up) and fix 01563_distributed_query_finish, 02435_rollback_cancelled_queries, 02345_implicit_transaction

@rschu1ze attribution and plan for the three tests:

02435_rollback_cancelled_queries — fix PR #105474. The 20 hits in 30 days are all on Stateless tests (arm_asan_ubsan, targeted) with stdout already correct (1000000\n0) and stderr full of ==NNNN==WARNING: ptrace appears to be blocked ... Child exited with signal 36. The test SIGKILLs clickhouse-client mid-INSERT; LSan's at-exit ptrace tracer cannot attach under the targeted runner's seccomp filter, falls back to SIGRTMIN+2, and the stderr warning trips Praktika's stderr-fatal watch. Setting LSAN_OPTIONS=detect_leaks=0 in the test makes the spawned clients skip the leak scan — running it on a process we are killing on purpose is not useful.

01563_distributed_query_finish — deferred to @Algunenano's open PR #105457. His approach is materially better than what I had drafted: wrap the loop in SYSTEM STOP/START DISTRIBUTED SENDS so the CI system.*_log_sender Distributed tables (set up by ci/jobs/scripts/functional_tests/setup_log_cluster.sh) stop shipping .bin files for the duration of the test. That kills the actual source of the background NETWORK_ERROR bursts (the cloud log endpoint being unreachable) instead of changing the measurement, and it preserves the bug-detection intent exactly. I won't open a competing PR.

02345_implicit_transaction — needs a server-side change. The failure is Code: 159 ... Timeout exceeded (180 s) while flushing system log and the 180 is a hardcoded const int timeout_seconds = 180 in SystemLogBase.cpp:164 (comment already notes 60s proved to be too fast for our parallel functional tests). SET receive_timeout doesn't help here — the throw is server-side from SystemLogQueue::waitFlush. CIDB shows 23 hits in 30 days across PRs #99854 (13), #104217 (7), #104947 (1), #104714 (1), exclusively when query_log backlog is heavy on parallel runners. Cleanest fix is to make that timeout a server setting (default still safe, but bumpable) or just raise the constant to ~600s; I have a follow-up task open to land that as a separate PR.

Session: cron:clickhouse-ci-task-worker:20260520-211500.

Third of the three flaky tests in this bundle is now in flight: #105484 — bumps the hardcoded SystemLogQueue::waitFlush timeout from 180s to 600s to fix 02345_implicit_transaction failing with TIMEOUT_EXCEEDED on parallel CI runners.

Bundle status:

• Skip LSan leak check in 02435_rollback_cancelled_queries #105474 — 02435_rollback_cancelled_queries (LSan leak check skip)
• tests: fix 01563_distributed_query_finish flakiness from CI log-sender bursts #105457 — 01563_distributed_query_finish (CI log-sender bursts) — @Algunenano
• Bump SystemLogQueue::waitFlush timeout to 600s #105484 — 02345_implicit_transaction (SystemLogBase timeout bump) — this one

CIDB pattern for 02345: 23 master hits across 4 distinct PRs over the last 30 days, all on parallel runners. The previous 60s -> 180s bump was made for the same reason; raising again is the same shape of fix.

#105457 — 01563_distributed_query_finish (CI log-sender bursts) — @Algunenano

@groeneai Please make a note to stop pinging me on unrelated PRs. When talking about flaky checks and its fixes, just mention the PR, no need to mention the author. Otherwise I get multiple pings that are completely unrelated to my work (I had half a dozen related to this exact fix I've made)

@Algunenano Noted, sorry for the noise. Going forward, when referencing a sibling fix PR in a thread the author isn't subscribed to, I'll use the bare PR number (e.g. #105457) instead of cross-tagging. The rule is memorized for future sessions.

@groeneai Make yourself useful (and Nvidia stocks go up) and fix 01563_distributed_query_finish, 02435_rollback_cancelled_queries, 02345_implicit_transaction

@rschu1ze is there anything else I can help with as the failures on PR seem to be unrelated? Basically we have to wait for the fixes to land on master, I can then rebase and when the checks pass we merge?

@antoniofilipovic All good, no need to do anything else from your side. I'll take over.

LLVM Coverage Report

Changed lines: 98.10% (103/105) | lost baseline coverage: 1 line(s) · Uncovered code

Full report · Diff report

Fix for 04103_user_network_bandwidth_throttler: #103586