Skip to content

Commit 76f9c32

Browse files
fix(tools): make Args the single source of truth for remote_write_ip (#548)
* fix(tools): make Args the single source of truth for remote_write_ip cfg.streaming.remote_write.ip has been a dead null since PR #469 replaced the OmegaConf resolver that used to populate it with a plain per-script `args.remote_write_ip = provider.get_node_ip(...)` — a value only arroyo.py ever read. generate_prometheus_config still reads cfg.streaming.remote_write.ip directly, so it silently baked http://None:<port>/... into prometheus.yml for every non-arroyo streaming engine, causing remote_write to fail and precompute-engine queries to hang forever waiting on data that never arrives (#546). Args.__init__ now builds the provider itself and sets both self.remote_write_ip and cfg.streaming.remote_write.ip from that one place, so every script gets it automatically instead of each one re-deriving (and potentially forgetting) it. Scripts that used to call create_provider(cfg) right alongside config.Args(cfg) now just read args.provider. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(tools): fix cmdline_args.txt dump after Args gained a provider field vars(args) broke once Args started holding a live provider object (added in the previous commit) — CloudLabProvider/LocalProvider aren't JSON-serializable, so every script crashed writing cmdline_args.txt. Args.to_dict() now swaps provider for its repr() (both provider classes already define one) instead of dropping it, keeping the debug dump both working and informative. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Updated scrape_interval to 1s, from 10s --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent cd0f271 commit 76f9c32

7 files changed

Lines changed: 39 additions & 32 deletions

File tree

‎asap-tools/experiments/config/config.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ streaming:
7070
# Prometheus configuration
7171
prometheus:
7272
# Global Prometheus settings
73-
scrape_interval: "10s" # How frequently to scrape targets
73+
scrape_interval: "1s" # How frequently to scrape targets
7474
evaluation_interval: "10s" # How frequently to evaluate rules
7575
# query_log_file: "/scratch/sketch_db_for_prometheus/prometheus/queries.log" # Disabled to avoid permission issues in Docker
7676
# Recording rules settings

‎asap-tools/experiments/experiment_only_ingest_path.py‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import constants
99
import experiment_utils
1010
from experiment_utils import sync, config
11-
from experiment_utils.providers.factory import create_provider
1211
from experiment_utils.services import (
1312
KafkaService,
1413
ExporterServiceFactory,
@@ -74,11 +73,9 @@ def main(cfg: DictConfig):
7473
is_v2 = experiment_mode == constants.SKETCHDB_EXPERIMENT_NAME
7574

7675
# Convert config to args-like object for backward compatibility
76+
# (also constructs the infrastructure provider, exposed as args.provider)
7777
args = config.Args(cfg)
78-
79-
# Create infrastructure provider
80-
provider = create_provider(cfg)
81-
args.remote_write_ip = provider.get_node_ip(args.node_offset)
78+
provider = args.provider
8279

8380
local_experiment_root_dir = os.path.join(
8481
constants.LOCAL_EXPERIMENT_DIR, args.experiment_name
@@ -91,7 +88,7 @@ def main(cfg: DictConfig):
9188

9289
# Also dump args to a file for backward compatibility
9390
with open(os.path.join(local_experiment_root_dir, "cmdline_args.txt"), "w") as f:
94-
json.dump(vars(args), f)
91+
json.dump(args.to_dict(), f)
9592

9693
experiment_root_output_dir = (
9794
f"{constants.CLOUDLAB_HOME_DIR}/experiment_outputs/{args.experiment_name}"

‎asap-tools/experiments/experiment_run_e2e.py‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import constants
99
import experiment_utils
1010
from experiment_utils import sync, config
11-
from experiment_utils.providers.factory import create_provider
1211
from experiment_utils.services import (
1312
KafkaService,
1413
FlinkService,
@@ -55,12 +54,10 @@ def main(cfg: DictConfig):
5554
# Validate experiment configuration
5655
config.validate_experiment_config(cfg.experiment_params)
5756

58-
# Create infrastructure provider
59-
provider = create_provider(cfg)
60-
6157
# Convert config to args-like object for backward compatibility
58+
# (also constructs the infrastructure provider, exposed as args.provider)
6259
args = config.Args(cfg)
63-
args.remote_write_ip = provider.get_node_ip(args.node_offset)
60+
provider = args.provider
6461

6562
if provider.is_remote():
6663
local_experiment_root_dir = os.path.join(
@@ -85,7 +82,7 @@ def main(cfg: DictConfig):
8582

8683
# Also dump args to a file for backward compatibility
8784
with open(os.path.join(local_experiment_root_dir, "cmdline_args.txt"), "w") as f:
88-
json.dump(vars(args), f)
85+
json.dump(args.to_dict(), f)
8986

9087
global CONTROLLER_REMOTE_OUTPUT_DIR, CONTROLLER_LOCAL_OUTPUT_DIR
9188
CONTROLLER_LOCAL_OUTPUT_DIR = os.path.join(

‎asap-tools/experiments/experiment_run_exporters_and_prometheus.py‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import constants
88
import experiment_utils
99
from experiment_utils import sync, config
10-
from experiment_utils.providers.factory import create_provider
1110
from experiment_utils.services import (
1211
ExporterServiceFactory,
1312
SystemExportersService,
@@ -30,10 +29,9 @@ def main(cfg: DictConfig):
3029
# Validate experiment configuration
3130
config.validate_experiment_config(cfg.experiment_params)
3231
# Convert config to args-like object for backward compatibility
32+
# (also constructs the infrastructure provider, exposed as args.provider)
3333
args = config.Args(cfg)
34-
35-
# Create infrastructure provider
36-
provider = create_provider(cfg)
34+
provider = args.provider
3735

3836
local_experiment_root_dir = os.path.join(
3937
constants.LOCAL_EXPERIMENT_DIR, args.experiment_name
@@ -46,7 +44,7 @@ def main(cfg: DictConfig):
4644

4745
# Also dump args to a file for backward compatibility
4846
with open(os.path.join(local_experiment_root_dir, "cmdline_args.txt"), "w") as f:
49-
json.dump(vars(args), f)
47+
json.dump(args.to_dict(), f)
5048

5149
experiment_root_output_dir = (
5250
f"{constants.CLOUDLAB_HOME_DIR}/experiment_outputs/{args.experiment_name}"

‎asap-tools/experiments/experiment_run_grafana_demo.py‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import constants
99
import experiment_utils
1010
from experiment_utils import sync, config
11-
from experiment_utils.providers.factory import create_provider
1211
from experiment_utils.services import (
1312
KafkaService,
1413
QueryEngineRustService,
@@ -51,11 +50,9 @@ def main(cfg: DictConfig):
5150
# Validate experiment configuration
5251
config.validate_experiment_config(cfg.experiment_params)
5352
# Convert config to args-like object for backward compatibility
53+
# (also constructs the infrastructure provider, exposed as args.provider)
5454
args = config.Args(cfg)
55-
56-
# Create infrastructure provider
57-
provider = create_provider(cfg)
58-
args.remote_write_ip = provider.get_node_ip(args.node_offset)
55+
provider = args.provider
5956

6057
args.forward_unsupported_queries = True
6158
print("Forcing forward_unsupported_queries to True for Grafana demo")
@@ -71,7 +68,7 @@ def main(cfg: DictConfig):
7168

7269
# Also dump args to a file for backward compatibility
7370
with open(os.path.join(local_experiment_root_dir, "cmdline_args.txt"), "w") as f:
74-
json.dump(vars(args), f)
71+
json.dump(args.to_dict(), f)
7572

7673
experiment_root_output_dir = (
7774
f"{constants.CLOUDLAB_HOME_DIR}/experiment_outputs/{args.experiment_name}"

‎asap-tools/experiments/experiment_teardown_everything.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
import constants
1616
from experiment_utils import config
17-
from experiment_utils.providers.factory import create_provider
1817
from experiment_utils.services import (
1918
KafkaService,
2019
FlinkService,
@@ -55,9 +54,7 @@ def main(cfg: DictConfig):
5554
# Validate configuration (minimal validation for provider setup)
5655
config.validate_config(cfg)
5756
args = config.Args(cfg)
58-
59-
# Create infrastructure provider
60-
provider = create_provider(cfg)
57+
provider = args.provider
6158

6259
num_nodes_in_experiment = args.num_nodes
6360

‎asap-tools/experiments/experiment_utils/config.py‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from omegaconf import DictConfig, ListConfig, OmegaConf
1212

1313
import constants
14+
from experiment_utils.providers.factory import create_provider
1415

1516

1617
def validate_basic_config(
@@ -532,6 +533,19 @@ def __init__(self, cfg: DictConfig):
532533
self.cloudlab_username = cfg.providers.cloudlab.username
533534
self.hostname_suffix = cfg.providers.cloudlab.hostname_suffix
534535

536+
# Single source of truth for the infrastructure provider: build it here
537+
# (from cfg alone) so every node-dependent value derived from it —
538+
# remote_write_ip included — is computed in exactly one place instead
539+
# of being re-derived (and potentially forgotten) in each script.
540+
self.provider = create_provider(cfg)
541+
542+
# Remote write IP (127.0.0.1 for local mode, CloudLab's 10.10.1.x
543+
# scheme otherwise). Written back onto cfg.streaming.remote_write.ip
544+
# too, since generate_prometheus_config reads it from cfg directly
545+
# rather than through this Args object.
546+
self.remote_write_ip = self.provider.get_node_ip(self.node_offset)
547+
cfg.streaming.remote_write.ip = self.remote_write_ip
548+
535549
# Logging and debugging
536550
self.log_level = cfg.logging.level
537551

@@ -565,10 +579,8 @@ def __init__(self, cfg: DictConfig):
565579
self.do_local_flink = cfg.streaming.do_local_flink
566580
self.forward_unsupported_queries = cfg.streaming.forward_unsupported_queries
567581
self.use_kafka_ingest = cfg.streaming.use_kafka_ingest
568-
# Remote write configuration
569-
self.remote_write_ip = (
570-
None # set by caller: provider.get_node_ip(self.node_offset)
571-
)
582+
# Remote write configuration (self.remote_write_ip set above, near
583+
# provider construction)
572584
self.remote_write_base_port = cfg.streaming.remote_write.base_port
573585
self.remote_write_path = cfg.streaming.remote_write.path
574586

@@ -624,6 +636,15 @@ def get_coordinator_node(self) -> int:
624636
"""Get the coordinator node index (first node in the range)."""
625637
return self.node_offset
626638

639+
def to_dict(self) -> Dict[str, Any]:
640+
"""JSON-serializable snapshot of this Args instance.
641+
642+
`provider` holds a live InfrastructureProvider object, so it's
643+
swapped for its repr() (e.g. "CloudLabProvider(username='...', ...)")
644+
rather than dropped.
645+
"""
646+
return {k: (repr(v) if k == "provider" else v) for k, v in vars(self).items()}
647+
627648

628649
def validate_config(cfg: DictConfig, script_name: str = "experiment_run_e2e"):
629650
"""

0 commit comments

Comments
 (0)