From e224559731bcf822c79129db13511bdc130aad93 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Tue, 7 Jul 2026 14:28:14 -0400 Subject: [PATCH] Pin TZ=UTC across ClickHouse/planner/query-engine experiment configs Naive datetime-string time literals in SQL queries were parsing inconsistently between ClickHouse (defaults to UTC) and asap-planner / query_engine_rust (local shell TZ), silently disagreeing by the shell's UTC offset. Also passes optional sketch_parameters through to the generated SQL planner input, and rsyncs init_sql_file to the remote node instead of assuming it's already there. --- asap-planner-rs/docker-compose.yml.j2 | 5 +++++ asap-query-engine/docker-compose.yml.j2 | 4 ++++ .../experiments/experiment_run_clickhouse.py | 8 ++++++-- .../experiments/experiment_utils/config.py | 16 +++++++++++++++- .../services/clickhouse_service.py | 12 +++++++++--- .../services/docker-compose.clickhouse.yml.j2 | 4 ++++ .../experiment_utils/services/misc.py | 7 ++++++- .../experiment_utils/services/query_engine.py | 11 ++++++++++- 8 files changed, 59 insertions(+), 8 deletions(-) diff --git a/asap-planner-rs/docker-compose.yml.j2 b/asap-planner-rs/docker-compose.yml.j2 index 301b5c0a..67055065 100644 --- a/asap-planner-rs/docker-compose.yml.j2 +++ b/asap-planner-rs/docker-compose.yml.j2 @@ -2,6 +2,11 @@ services: controller: image: sketchdb-controller:latest container_name: {{ container_name }} + environment: + # Naive datetime-string time literals in SQL queries must parse the + # same way here as in ClickHouse (UTC by default) and in bare-metal + # asap-planner (see experiment_utils/services/misc.py). + - TZ=UTC volumes: - {{ input_config_path }}:/app/input/config.yaml:ro - {{ output_dir }}:/app/output diff --git a/asap-query-engine/docker-compose.yml.j2 b/asap-query-engine/docker-compose.yml.j2 index ea9879fc..e5dbec10 100644 --- a/asap-query-engine/docker-compose.yml.j2 +++ b/asap-query-engine/docker-compose.yml.j2 @@ -10,6 +10,10 @@ services: environment: - RUST_LOG={{ log_level }} - RUST_BACKTRACE=1 + # Naive datetime-string time literals in incoming SQL queries must parse + # the same way here as in ClickHouse (UTC by default) and in bare-metal + # query_engine_rust (see experiment_utils/services/query_engine.py). + - TZ=UTC ports: - "{{ http_port }}:8088" network_mode: "host" diff --git a/asap-tools/experiments/experiment_run_clickhouse.py b/asap-tools/experiments/experiment_run_clickhouse.py index 7bdd370d..5e9ac7f2 100644 --- a/asap-tools/experiments/experiment_run_clickhouse.py +++ b/asap-tools/experiments/experiment_run_clickhouse.py @@ -93,6 +93,7 @@ import json import os import time +from typing import Optional from urllib.parse import urlparse import hydra @@ -162,7 +163,7 @@ def _run_query_workload( remote_monitor_service: RemoteMonitorService, minimum_experiment_running_time: int, manual_remote_monitor: bool, - query_engine_service: QueryEngineRustService | None, + query_engine_service: Optional[QueryEngineRustService], profile_query_engine: bool, profile_prometheus_time, ) -> None: @@ -300,6 +301,9 @@ def main(cfg: DictConfig) -> None: local_experiment_dir=local_experiment_root_dir, http_port=clickhouse_http_port, database=CLICKHOUSE_DATABASE, + # Pre-AVX2 CPUs (e.g. Sandy/Ivy Bridge CloudLab nodes) SIGILL on the + # "latest" image; override via dataset.clickhouse_image_tag if needed. + image_tag=dataset_cfg.get("clickhouse_image_tag", "latest"), ) # --- load data once before the mode loop (DROP + reload) --- @@ -367,7 +371,7 @@ def main(cfg: DictConfig) -> None: # Generate and rsync the planner input config to the node planner_input_yaml = config.generate_sql_planner_input( - ep.query_groups, dataset_cfg + ep.query_groups, dataset_cfg, cfg.get("sketch_parameters", None) ) local_planner_input = os.path.join( local_controller_dir, "planner_input.yaml" diff --git a/asap-tools/experiments/experiment_utils/config.py b/asap-tools/experiments/experiment_utils/config.py index af0d59d6..930e4dee 100644 --- a/asap-tools/experiments/experiment_utils/config.py +++ b/asap-tools/experiments/experiment_utils/config.py @@ -765,13 +765,19 @@ def generate_clickhouse_client_configs( return modes -def generate_sql_planner_input(query_groups: Any, dataset_cfg: Any) -> str: +def generate_sql_planner_input( + query_groups: Any, dataset_cfg: Any, sketch_parameters: Any = None +) -> str: """Generate the YAML input file for asap-planner in SQL mode. The planner (``asap-planner --query-language sql``) reads a ``SQLControllerConfig`` YAML that contains: - ``tables``: schema of the tables being queried - ``query_groups``: SQL queries with controller options + - ``sketch_parameters``: optional per-sketch-type overrides (e.g. + ``DatasketchesKLL.K``), matching ``ControllerConfig``'s PromQL-mode + field of the same name (``SketchParameterOverrides`` in + asap-planner-rs's ``config/input.rs``). This function builds that YAML from the experiment config so the runner does not need a hand-authored planner input file. @@ -782,6 +788,10 @@ def generate_sql_planner_input(query_groups: Any, dataset_cfg: Any) -> str: ``controller_options`` (``accuracy_sla``, ``latency_sla``). dataset_cfg: DictConfig with ``table``/``name``, and ``precompute`` sub-config (``timestamp_col``, ``value_col``, ``label_cols``). + sketch_parameters: Optional DictConfig/dict mirroring ``config.yaml``'s + top-level ``sketch_parameters`` section (``CountMinSketch``, + ``DatasketchesKLL``, etc.). When ``None``, the planner falls back + to its own defaults. Returns: YAML string ready to write to disk and pass to asap-planner. @@ -830,6 +840,10 @@ def generate_sql_planner_input(query_groups: Any, dataset_cfg: Any) -> str: "query_groups": planner_query_groups, "aggregate_cleanup": {"policy": "read_based"}, } + if sketch_parameters is not None: + if isinstance(sketch_parameters, (DictConfig, ListConfig)): + sketch_parameters = OmegaConf.to_container(sketch_parameters, resolve=True) + planner_input["sketch_parameters"] = sketch_parameters return yaml.dump(planner_input, default_flow_style=False, allow_unicode=True) diff --git a/asap-tools/experiments/experiment_utils/services/clickhouse_service.py b/asap-tools/experiments/experiment_utils/services/clickhouse_service.py index a423c944..e79dd925 100644 --- a/asap-tools/experiments/experiment_utils/services/clickhouse_service.py +++ b/asap-tools/experiments/experiment_utils/services/clickhouse_service.py @@ -268,8 +268,9 @@ def start( table: Target table name. Defaults to the dataset's standard table (``hits`` for clickbench, ``h2o_groupby`` for h2o). batch_size: INSERT batch size for H2O loading (default 50 000). - init_sql_file: Path to a DDL SQL file *already on the remote node*. - When ``None``, the built-in ``*_init.sql`` is rsynced and used. + init_sql_file: Path to a local DDL SQL file; rsynced to the remote + node and executed. When ``None``, the built-in ``*_init.sql`` + is rsynced and used instead. max_rows: Maximum rows to load (0 = all). """ if remote_data_file is None: @@ -292,7 +293,12 @@ def start( if init_sql_file is not None: print(f"Running init SQL from {init_sql_file!r}...") - self._exec_sql_file(init_sql_file, url) + remote_ddl = f"/tmp/{dataset_name}_init_{os.getpid()}.sql" + self._rsync_to_remote(init_sql_file, remote_ddl) + try: + self._exec_sql_file(remote_ddl, url) + finally: + self._remote_rm(remote_ddl) elif dataset_name in self.BUILTIN_DDL_FILES: local_ddl = os.path.join( self._ASSETS_DIR, self.BUILTIN_DDL_FILES[dataset_name] diff --git a/asap-tools/experiments/experiment_utils/services/docker-compose.clickhouse.yml.j2 b/asap-tools/experiments/experiment_utils/services/docker-compose.clickhouse.yml.j2 index 0462fb31..9fa65ef6 100644 --- a/asap-tools/experiments/experiment_utils/services/docker-compose.clickhouse.yml.j2 +++ b/asap-tools/experiments/experiment_utils/services/docker-compose.clickhouse.yml.j2 @@ -11,6 +11,10 @@ services: environment: - CLICKHOUSE_DB={{ database }} - CLICKHOUSE_DEFAULT_ACCESS_MANAGEMENT=1 + # Explicit rather than relying on the image's default: naive datetime- + # string time literals in SQL queries must parse identically here and + # in asap-planner/query_engine_rust (both pinned to TZ=UTC too). + - TZ=UTC ulimits: nofile: soft: 262144 diff --git a/asap-tools/experiments/experiment_utils/services/misc.py b/asap-tools/experiments/experiment_utils/services/misc.py index fdb398eb..1142762f 100644 --- a/asap-tools/experiments/experiment_utils/services/misc.py +++ b/asap-tools/experiments/experiment_utils/services/misc.py @@ -257,8 +257,13 @@ def _start_bare_metal( query_language: str, ) -> None: controller_log = os.path.join(controller_remote_output_dir, "controller.log") + # Force UTC so naive (no Z/offset) datetime-string time literals in SQL + # queries parse identically here (parse_datetime, sqlpattern_parser.rs) + # and in ClickHouse (whose container has no TZ override, so it defaults + # to UTC) -- otherwise the two would silently disagree by the shell's + # local UTC offset. cmd = ( - f"../target/release/asap-planner" + f"TZ=UTC ../target/release/asap-planner" f" --input_config {controller_input_file}" f" --output_dir {controller_remote_output_dir}" f" --streaming_engine {streaming_engine}" diff --git a/asap-tools/experiments/experiment_utils/services/query_engine.py b/asap-tools/experiments/experiment_utils/services/query_engine.py index 092a6ea4..88f49d24 100644 --- a/asap-tools/experiments/experiment_utils/services/query_engine.py +++ b/asap-tools/experiments/experiment_utils/services/query_engine.py @@ -333,8 +333,17 @@ def _start_bare_metal( cmd_dir = os.path.join( self.provider.get_home_dir(), "code", "asap-query-engine" ) + # Force UTC so naive datetime-string time literals in incoming SQL + # queries parse the same way here as in ClickHouse (UTC by default) + # and in asap-planner (see misc.py's ControllerService for the same + # fix) -- otherwise absolute-timestamp queries could silently + # disagree by the shell's local UTC offset. cmd = ( - f"../target/release/query_engine_rust" + # `env` (not a bare `TZ=UTC` prefix) since this runs under nohup, + # which execs argv[0] directly rather than re-parsing through a + # shell -- a bare `VAR=val` prefix would make nohup try (and + # fail) to exec "TZ=UTC" itself as the program name. + f"env TZ=UTC ../target/release/query_engine_rust" f" --config-file {output_dir}/engine_config.yaml" f" > {output_dir}/query_engine_rust.out 2>&1 &" )