Keep distributed_plan_workers_provisioning_timeout_ms at 10s under compatibility - #118996
Conversation
…compatibility The settings history recorded this new setting with previous_value 0, so `compatibility` below 26.9 restored the pre-feature no-wait behavior (only the initial lease-and-verify pass, no retries). The wait-and-retry provisioning is brand-new functionality with no released default to preserve, and restoring the no-wait behavior only makes worker provisioning less robust. Set the recorded previous value equal to the new default (10000) so that `compatibility` keeps the 10s budget in all modes, matching the established pattern for new settings that should not be rolled back by `compatibility`.
|
Workflow [PR], commit [e2eb1f7] Summary: ✅
AI ReviewSummaryThis PR changes the Findings❌ Blockers
Tests
Final Verdict❌ Not ready to merge. The fix needs to preserve the released LLVM Coverage ReportMeasured on commit e2eb1f7.
Changed lines: Changed C/C++ lines covered: 7/7 (100.00%) · Uncovered code |
clickhouse-gh
Bot
added
the
pr-not-for-changelog
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
| {"force_write_through_distributed_cache", "auto", "auto", "New setting overriding the server setting `enable_write_through_distributed_cache` for a single query."}, | ||
| {"distributed_cache_min_inflight_bytes_to_discard_connection_on_seek", 0, 4 * 1024 * 1024, "New setting to drop and reopen a distributed cache connection on a seek when too many in-flight bytes would otherwise be discarded. Defaults to 4 MiB; 0 restores the previous behavior (always reuse the connection via the read range id)."}, | ||
| {"distributed_plan_workers_provisioning_timeout_ms", 0, 10000, "New setting bounding how long a query waits for leased stateless workers to become reachable before execution; `compatibility` below 26.9 restores the previous no-wait behavior."}, | ||
| {"distributed_plan_workers_provisioning_timeout_ms", 10000, 10000, "New setting bounding how long a query waits for leased stateless workers to become reachable before execution. The previous value is deliberately equal to the new default so `compatibility` does not restore the pre-feature no-wait behavior: the wait is brand-new functionality with no released default to preserve, and restoring the no-wait behavior would only make worker provisioning less robust."}, |
There was a problem hiding this comment.
compatibility is supposed to emulate the first released version that shipped a behavior, not the current master shape. Released v26.8.2.7-lts already ships distributed_plan_workers_provisioning_timeout_ms = 10000, and its SettingsChangesHistory.cpp records that rollback boundary in the 26.8 block with previous_value = 0. Changing the 26.9 copy to {10000, 10000} fixes compatibility = '26.8', but it also stops compatibility = '26.7' and older from restoring the legacy no-wait behavior.
So this needs a version-boundary fix, not a permanent no-op: move the row to the 26.8 block with previous_value = 0, and pin it with a focused stateless test for compatibility = '26.8' versus compatibility = '26.7'.
clickhouse-gh
Bot
added
the
comp-settings
Build profile diff (arm_release)Comparing ✅ No significant changes. Binary sizes
The official master build is compiled with Compile time of recompiled translation units8 translation units recompiled, 11 s compile time in total, 8 of them have a recent master baseline. |
robot-clickhouse-ci-1
added
the
pr-synced-to-cloud
3 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Description
distributed_plan_workers_provisioning_timeout_ms(default10000) was recorded in the settings history as a change from0to10000, socompatibilitybelow26.9rolled it back to0. Per the setting's own description,0makes a query wait only for the initial lease-and-verify pass, with no retries — the pre-feature no-wait behavior.The wait-and-retry provisioning is brand-new functionality with no previously released default to preserve, so restoring
0undercompatibilityhas no backward-compatibility benefit and only makes worker provisioning less robust (a query can give up on workers that would have become reachable within the 10s budget).This sets the recorded previous value equal to the new default (
10000) socompatibilitykeeps the 10s budget in all modes, matching the established pattern for new settings that should not be rolled back bycompatibility(for examplestatistics_max_set_size_for_exact_selectivity_estimation, whose entry is{10000, 10000}for the same reason).Only
src/Core/SettingsChangesHistory.cppchanges; the compiled default is unchanged.Workflow [PR]
Sync PR [sync-upstream/pr/118996]