Skip to content

Commit 4138a7f

Browse files
zzylolclaudemilindsrivastava1997
authored
fix(asap-tools): run cluster-trace e2e experiments with the local provider (#751)
* fix(asap-tools): run cluster_data_exporter in local mode and find Alibaba data files The service asserted exactly one worker node, which local mode (0 nodes) never satisfies, and validated Alibaba data against MsResource_*.csv.gz, a name the exporter never reads, so valid MSMetrics/NodeMetrics data was rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(asap-tools): run e2e experiments with the local provider The exporter health poll aborted on the first failed curl because the local provider raises on non-zero exits, and the monitor keyword list kept its SSH-escaped quotes locally, so no process matched and the query client never ran. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(asap-tools): keep series that miss some repetitions in fidelity results get_all_timeseries raised KeyError when a key was absent from one repetition, e.g. a series that appears mid-run, so calculate_fidelity crashed on cluster-trace runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(asap-tools): pass ms-resource to the Alibaba cluster_data_exporter The exporter's --data-type accepts node or ms-resource; the config value msresource was passed through, so the container exited at startup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(asap-tools): unescaped monitor keywords for local ClickHouse ingest start_clickhouse_ingest_monitor hardcoded the SSH-only escaped quoting, so local runs passed "clickhouse" with literal quotes and monitored nothing. Share the quoting with start() via _keywords_arg. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Milind Srivastava <milindsrivastava1997@gmail.com>
1 parent 6d3af52 commit 4138a7f

5 files changed

Lines changed: 199 additions & 30 deletions

File tree

‎asap-common/dependencies/py/promql_utilities/promql_utilities/query_results/classes.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,10 @@ def get_all_timeseries(self) -> Dict[frozenset, TimeSeries]:
6565

6666
for k in keys:
6767
for repetition_idx, result in enumerate(self.query_results):
68+
# A key absent from one repetition (e.g. a series that appears
69+
# mid-run) stays None for that repetition.
6870
if result.result:
69-
intermediate_ret[k][repetition_idx] = result.result[k]
71+
intermediate_ret[k][repetition_idx] = result.result.get(k)
7072

7173
ret[k] = TimeSeries(k, intermediate_ret[k])
7274

‎asap-tools/experiments/experiment_utils/services/cluster_data_exporter.py‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@
1313
from experiment_utils.providers.base import InfrastructureProvider
1414

1515

16+
# Experiment-config data_type -> the exporter's --data-type value.
17+
ALIBABA_DATA_TYPE_CLI_VALUES = {"node": "node", "msresource": "ms-resource"}
18+
19+
1620
class ClusterDataExporterService(BaseService):
1721
"""
1822
Service for managing cluster_data_exporter via Docker.
@@ -71,10 +75,11 @@ def start(
7175
# Get number of nodes from provider (assuming it has this info)
7276
num_nodes = kwargs.get("num_nodes", 1)
7377

74-
# Assert that we have exactly 2 nodes
75-
assert num_nodes == 1, (
76-
f"cluster_data_exporter requires exactly 1 node (num_nodes==1), "
77-
f"got {num_nodes}"
78+
# One worker node next to the coordinator; local mode reports 0 nodes
79+
# and runs everything on this machine.
80+
assert num_nodes in (0, 1), (
81+
f"cluster_data_exporter requires one worker node (num_nodes==1) "
82+
f"or local mode (num_nodes==0), got {num_nodes}"
7883
)
7984

8085
# Extract configuration
@@ -258,7 +263,8 @@ def _build_docker_command(
258263

259264
elif provider == "alibaba":
260265
if "data_type" in config:
261-
cmd_parts.append(f"--data-type={config['data_type']}")
266+
data_type = ALIBABA_DATA_TYPE_CLI_VALUES[config["data_type"]]
267+
cmd_parts.append(f"--data-type={data_type}")
262268
if "data_year" in config:
263269
cmd_parts.append(f"--data-year={config['data_year']}")
264270

@@ -400,23 +406,18 @@ def _validate_alibaba_data(
400406
Raises:
401407
ValueError: If required files are missing
402408
"""
403-
# Determine expected file pattern based on data type and year
404-
if data_type == "node":
405-
if data_year == 2021 or data_year == 2022:
406-
pattern = "Node_*.csv.gz"
407-
else:
408-
raise ValueError(
409-
f"Invalid data_year for Alibaba node data: {data_year}"
410-
)
411-
elif data_type == "msresource":
412-
if data_year == 2021 or data_year == 2022:
413-
pattern = "MsResource_*.csv.gz"
414-
else:
415-
raise ValueError(
416-
f"Invalid data_year for Alibaba msresource data: {data_year}"
417-
)
418-
else:
419-
raise ValueError(f"Invalid data_type for Alibaba: {data_type}")
409+
# File names the exporter reads (alibaba_metrics/{node,ms_resource}.rs).
410+
patterns = {
411+
("node", 2021): "Node_*.csv.gz",
412+
("node", 2022): "NodeMetrics_*.csv.gz",
413+
("msresource", 2021): "MSResource_*.csv.gz",
414+
("msresource", 2022): "MSMetrics_*.csv.gz",
415+
}
416+
pattern = patterns.get((data_type, int(data_year)))
417+
if pattern is None:
418+
raise ValueError(
419+
f"Invalid Alibaba data_type/data_year: {data_type}/{data_year}"
420+
)
420421

421422
# Check for data files on remote node
422423
target_node = self.node_offset + 1
@@ -497,12 +498,15 @@ def _wait_for_health(
497498
while time.time() - start_time < timeout:
498499
# Run curl from the remote node to check health
499500
check_cmd = f"curl -s -o /dev/null -w '%{{http_code}}' {url}"
501+
# curl exits non-zero while the exporter is still starting; the
502+
# local provider raises on that, so keep polling instead.
500503
result = self.provider.execute_command(
501504
node_idx=node_idx,
502505
cmd=check_cmd,
503506
cmd_dir="",
504507
nohup=False,
505508
popen=False,
509+
ignore_errors=True,
506510
)
507511

508512
try:

‎asap-tools/experiments/experiment_utils/services/remote_monitor_service.py‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import shlex
88
import time
99
import subprocess
10-
from typing import Optional
10+
from typing import List, Optional
1111

1212
import constants
1313
from .base import BaseService
@@ -100,13 +100,15 @@ def start(
100100
else:
101101
keywords.append(constants.QUERY_ENGINE_RS_PROCESS_KEYWORD)
102102

103+
keywords_arg = self._keywords_arg(keywords)
104+
103105
if use_timed_mode:
104106
# Build command for timed mode (skip_querying)
105107
cmd = (
106108
"python3 -u remote_monitor.py "
107109
"--execution_mode timed "
108110
"--experiment_mode {} "
109-
r"--keywords \"{}\" "
111+
"--keywords {} "
110112
"--config_file {} "
111113
"--experiment_output_dir {} "
112114
"--monitor_output_file {} "
@@ -116,7 +118,7 @@ def start(
116118
"--monitor_interval_seconds {} "
117119
).format(
118120
experiment_mode,
119-
",".join(keywords),
121+
keywords_arg,
120122
os.path.join(
121123
os.path.dirname(experiment_output_dir),
122124
"controller_client_configs",
@@ -162,7 +164,7 @@ def start(
162164
"python3 -u remote_monitor.py "
163165
"--execution_mode prometheus_client "
164166
"--experiment_mode {} "
165-
r"--keywords \"{}\" "
167+
"--keywords {} "
166168
"--config_file {} "
167169
"--experiment_output_dir {} "
168170
"--monitor_output_file {} "
@@ -172,7 +174,7 @@ def start(
172174
"--monitor_interval_seconds {} "
173175
).format(
174176
experiment_mode,
175-
",".join(keywords),
177+
keywords_arg,
176178
os.path.join(
177179
os.path.dirname(experiment_output_dir),
178180
"controller_client_configs",
@@ -261,7 +263,7 @@ def start_clickhouse_ingest_monitor(
261263
"python3 -u remote_monitor.py "
262264
"--execution_mode ingest "
263265
"--experiment_mode {} "
264-
r"--keywords \"{}\" "
266+
"--keywords {} "
265267
"--config_file {} "
266268
"--experiment_output_dir {} "
267269
"--monitor_output_file {} "
@@ -271,7 +273,7 @@ def start_clickhouse_ingest_monitor(
271273
"--backend_protocol clickhouse "
272274
).format(
273275
constants.BASELINE_EXPERIMENT_NAME,
274-
",".join(keywords),
276+
self._keywords_arg(keywords),
275277
controller_client_config,
276278
experiment_output_dir,
277279
monitor_output_file,
@@ -313,6 +315,13 @@ def start_clickhouse_ingest_monitor(
313315
popen=False,
314316
)
315317

318+
def _keywords_arg(self, keywords: List[str]) -> str:
319+
# Over SSH the command is parsed by two shells, so the quotes that keep
320+
# the keyword list one argument must be escaped once more.
321+
if self.provider.is_remote():
322+
return r"\"{}\"".format(",".join(keywords))
323+
return '"{}"'.format(",".join(keywords))
324+
316325
@staticmethod
317326
def _remote_monitor_pgrep_pattern(
318327
execution_mode: Optional[str] = None,
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
"""Tests for ClusterDataExporterService data-file validation and node checks."""
2+
3+
import subprocess
4+
import unittest
5+
6+
from experiment_utils.services.cluster_data_exporter import ClusterDataExporterService
7+
8+
9+
class RecordingProvider:
10+
"""Answers every command with one matching file and records the commands."""
11+
12+
def __init__(self):
13+
self.commands = []
14+
15+
def execute_command(self, node_idx, cmd, cmd_dir, nohup, popen):
16+
self.commands.append(cmd)
17+
return subprocess.CompletedProcess([], 0, "1\n", "")
18+
19+
20+
class ValidateAlibabaDataTest(unittest.TestCase):
21+
def _counted_pattern(self, data_type, data_year):
22+
provider = RecordingProvider()
23+
service = ClusterDataExporterService(provider, 0, "/traces")
24+
service._validate_alibaba_data(data_type, data_year)
25+
return [c for c in provider.commands if "wc -l" in c][0]
26+
27+
def test_patterns_match_exporter_file_names(self):
28+
# The check used to look for MsResource_*.csv.gz for both years, a
29+
# name the exporter never reads, so valid data was rejected.
30+
cases = {
31+
("node", 2021): "Node_*.csv.gz",
32+
("node", 2022): "NodeMetrics_*.csv.gz",
33+
("msresource", 2021): "MSResource_*.csv.gz",
34+
("msresource", 2022): "MSMetrics_*.csv.gz",
35+
}
36+
for (data_type, data_year), pattern in cases.items():
37+
with self.subTest(data_type=data_type, data_year=data_year):
38+
self.assertIn(
39+
f"/traces/{pattern}", self._counted_pattern(data_type, data_year)
40+
)
41+
42+
def test_unknown_year_raises(self):
43+
service = ClusterDataExporterService(RecordingProvider(), 0, "/traces")
44+
with self.assertRaises(ValueError):
45+
service._validate_alibaba_data("msresource", 2020)
46+
47+
48+
class DockerCommandTest(unittest.TestCase):
49+
def test_msresource_uses_exporter_cli_value(self):
50+
# The exporter's clap enum spells it ms-resource; passing the config
51+
# value through made the container exit before serving metrics.
52+
service = ClusterDataExporterService(RecordingProvider(), 0, "/traces")
53+
service.container_name = "cde"
54+
cmd = service._build_docker_command(
55+
{"provider": "alibaba", "data_type": "msresource", "data_year": 2022},
56+
port=40000,
57+
output_dir="/out",
58+
)
59+
self.assertIn("--data-type=ms-resource", cmd)
60+
61+
62+
class NodeCountTest(unittest.TestCase):
63+
def test_more_than_one_worker_node_is_rejected(self):
64+
service = ClusterDataExporterService(RecordingProvider(), 0, "/traces")
65+
with self.assertRaises(AssertionError):
66+
service.start({"provider": "google"}, "/out", "/local", num_nodes=2)
67+
68+
69+
if __name__ == "__main__":
70+
unittest.main()
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
"""Tests that remote_monitor.py receives its keyword list as one clean argument."""
2+
3+
import shlex
4+
import unittest
5+
6+
from experiment_utils.services.remote_monitor_service import RemoteMonitorService
7+
8+
9+
class RecordingProvider:
10+
def __init__(self, remote):
11+
self.remote = remote
12+
self.commands = []
13+
14+
def is_remote(self):
15+
return self.remote
16+
17+
def get_home_dir(self):
18+
return "/home"
19+
20+
def execute_command(self, **kwargs):
21+
self.commands.append(kwargs["cmd"])
22+
23+
24+
def keywords_seen_by_monitor(remote):
25+
provider = RecordingProvider(remote)
26+
RemoteMonitorService(provider, 0).start(
27+
controller_client_config="/out/controller_client_configs/baseline.yaml",
28+
experiment_output_dir="/out/baseline",
29+
experiment_mode="baseline",
30+
profile_query_engine=False,
31+
profile_prometheus_time=None,
32+
manual_mode=False,
33+
streaming_engine="precompute",
34+
query_engine_service=None,
35+
controller_remote_output_dir="/out/controller_output",
36+
use_container_prometheus_client=True,
37+
prometheus_client_parallel=False,
38+
backend_protocol="prometheus",
39+
pre_query_wait_seconds=0,
40+
monitor_interval_seconds=1.0,
41+
timed_duration=10,
42+
)
43+
args = shlex.split(provider.commands[0])
44+
if remote:
45+
# The SSH provider wraps the command in one more shell.
46+
args = shlex.split(" ".join(args))
47+
return args[args.index("--keywords") + 1]
48+
49+
50+
def clickhouse_ingest_keywords_seen_by_monitor(remote):
51+
provider = RecordingProvider(remote)
52+
RemoteMonitorService(provider, 0).start_clickhouse_ingest_monitor(
53+
controller_client_config="/out/controller_client_configs/baseline.yaml",
54+
experiment_output_dir="/out/baseline",
55+
)
56+
# The first command clears the stop file; the last launches the monitor.
57+
args = shlex.split(provider.commands[-1])
58+
if remote:
59+
args = shlex.split(" ".join(args))
60+
return args[args.index("--keywords") + 1]
61+
62+
63+
class KeywordQuotingTest(unittest.TestCase):
64+
def test_local_provider_gets_unescaped_keywords(self):
65+
# Escaped quotes meant for SSH used to reach remote_monitor.py
66+
# verbatim in local mode, so no process matched and no queries ran.
67+
self.assertEqual(keywords_seen_by_monitor(remote=False), "prometheus.yml")
68+
69+
def test_remote_provider_keeps_escaped_quotes_for_ssh(self):
70+
self.assertEqual(keywords_seen_by_monitor(remote=True), "prometheus.yml")
71+
72+
def test_clickhouse_ingest_local_provider_gets_unescaped_keywords(self):
73+
self.assertEqual(
74+
clickhouse_ingest_keywords_seen_by_monitor(remote=False), "clickhouse"
75+
)
76+
77+
def test_clickhouse_ingest_remote_provider_keeps_escaped_quotes_for_ssh(self):
78+
self.assertEqual(
79+
clickhouse_ingest_keywords_seen_by_monitor(remote=True), "clickhouse"
80+
)
81+
82+
83+
if __name__ == "__main__":
84+
unittest.main()

0 commit comments

Comments
 (0)