mirror of
https://github.com/hanzoai/datastore.git
synced 2026-09-26 13:55:07 +00:00
Alexey MilovidovandClaude Opus 4.8
345bf29095
The automatic value of settings like `max_threads` shows up in `system.settings` as `'auto(64)'` - with literal single quotes baked right into the string value. They are pure garbage and have annoyed me for years. Back in 2023 I removed them in a commit titled, with great restraint, "Fix strange trash". Konstantin promptly put the trash back in #68979 (branch `put-strange-trash-back`, commit "Revert \"Fix strange trash\""). And, in fairness, he was right at the time: the parser only understood the quoted form, so a new un-quoted server could not parse the `'auto(N)'` that older replicas send over the wire as a setting string, which broke mixed-version clusters (#68748). That obstacle is now gone. `stringToMaxThreads` accepts both `auto(N)` and the legacy `'auto(N)'`, so emitting the clean form is safe in both directions: an old replica's quoted value is still parsed by us, and every released version can already parse the un-quoted value we now send. So the strange trash goes back out - and this time the tolerant parser stays, so it stays out for good. Changes: - `SettingFieldMaxThreads::toString` now returns `auto(N)` without quotes. - Documented why `stringToMaxThreads` must keep accepting the legacy quoted form (it is the mixed-version-cluster compatibility guarantee). - Updated test `04202_set_default_preserves_max_threads_auto` and the `test_cgroup_limit` integration test to match the clean form. - The upgrade check (`upgrade_runner.sh`) now ignores `auto(...)` values when diffing settings against the previous release, so the quoted-vs-unquoted rendering is not reported as a setting change. - Updated the `system.settings` example in the docs. Related: https://github.com/ClickHouse/ClickHouse/issues/68748 Related: https://github.com/ClickHouse/ClickHouse/pull/68979 Related: https://github.com/ClickHouse/ClickHouse/pull/52503 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
59 lines
1.5 KiB
Python
59 lines
1.5 KiB
Python
import logging
|
|
import math
|
|
import os
|
|
import subprocess
|
|
|
|
from helpers.test_tools import wait_condition
|
|
|
|
|
|
def run_command_in_container(cmd, *args):
|
|
# /clickhouse is mounted by integration tests runner
|
|
alternative_binary = os.getenv("CLICKHOUSE_BINARY", "/clickhouse")
|
|
if alternative_binary:
|
|
args += (
|
|
"--volume",
|
|
f"{alternative_binary}:/usr/bin/clickhouse",
|
|
)
|
|
|
|
command = [
|
|
"docker",
|
|
"run",
|
|
"--rm",
|
|
*args,
|
|
"ubuntu:22.04",
|
|
"sh",
|
|
"-c",
|
|
cmd,
|
|
]
|
|
|
|
logging.debug("Command: %s", " ".join(command))
|
|
return subprocess.check_output(command)
|
|
|
|
|
|
def run_with_cpu_limit(cmd, num_cpus, *args):
|
|
args += (
|
|
"--cpus",
|
|
f"{num_cpus}",
|
|
)
|
|
return run_command_in_container(cmd, *args)
|
|
|
|
|
|
def test_cgroup_cpu_limit():
|
|
for num_cpus in (1, 2, 4, 2.8):
|
|
def run_with_retry():
|
|
result = run_with_cpu_limit(
|
|
"clickhouse local -q \"select value from system.settings where name='max_threads'\"",
|
|
num_cpus,
|
|
)
|
|
expect_output = (r"auto({})".format(math.ceil(num_cpus))).encode()
|
|
return (
|
|
result.strip() == expect_output
|
|
), f"fail for cpu limit={num_cpus}, result={result.strip()}, expect={expect_output}"
|
|
|
|
wait_condition(run_with_retry, lambda x:x, max_attempts=3, delay=0.2)
|
|
|
|
|
|
# For manual run
|
|
if __name__ == "__main__":
|
|
test_cgroup_cpu_limit()
|