feat(asap-tools): separate clickhouse ingest and query cpu/memory monitoring in experiment run clickhouse

[akanksha-akkihal]

Summary

• Separate ClickHouse bulk-ingest CPU/memory monitoring from the query-phase monitor in experiment_run_clickhouse.
• Baseline ingest writes baseline/remote_monitor_output/monitor_output_ingest.json and stops via a stop file (with kill fallback); query phase still writes monitor_output.json.
• SketchDB keeps a single continuous monitor that waits flow.steady_state_wait before the query client so precompute + query land in one time series (with bare-metal thread attribution).
• Harden stop/matching: poll during process walks, escalate stuck samplers with kill(), match remote_monitor.py by --execution_mode + experiment dir, and avoid writing null monitor JSON on timeout.

[milindsrivastava1997]

Does experiment_run_e2e.py need any changes? Has that script been run once with the new remote monitor to verify?

[milindsrivastava1997]

Comments from CR with Claude:

Hard violations

1. remote_monitor.py — the new ingest execution-mode installs global SIGTERM/SIGINT handlers that just set a threading.Event() and never exit/restore. This means the SIGTERM the PR's own new kill-escalation path sends to stuck samplers gets swallowed, so the "force
   kill" safety net it adds can itself hang.
2. remote_monitor_service.py::wait_for_remote_monitor_to_finish — gained an unconditional timeout=600 + force-kill escalation with no execution_mode/dir filter by default. This method is also called from the untouched experiment_run_e2e.py, so its "wait forever"
   semantics silently become "pkill any process matching remote_monitor.py after 10 min" for callers outside this PR's scope.

Magic numbers

3. constants.py was touched (added INGEST_MONITOR_STOP_FILE) but the new timeout/poll literals (30, 60, 300, 10, 5, 2, 0.5s) scattered across the three other files stay inline, breaking the file's existing pattern of named interval constants.
4. process_monitor.stop_monitor(..., timeout=30) only threads timeout into the first poll; the two .join() calls hardcode 10 and the kill escalation hardcodes 5 — inconsistent within one new function.

Implemented but wrong

5. Manual mode is broken for the ingest monitor. _run_clickhouse_ingest_with_monitor unconditionally calls wait_for_remote_monitor_start(...) right after start_clickhouse_ingest_monitor(..., manual_mode=...), but unlike existing manual-mode call sites in this file
   (which guard with if not manual_remote_monitor), the ingest path never pauses for the user — so cfg.manual.remote_monitor=true will always raise RuntimeError after 30s, before the user can react. This directly contradicts the "harden" intent of the PR by breaking an
   existing workflow.
6. Kill-fallback can cause total data loss. Because the monitor is a forked multiprocessing.Process, pkill -f in kill_remote_monitor() can SIGTERM the sampler child directly before the control_pipe stop handshake completes, leaving monitor_info=None. The null-JSON
   guard then deletes any partial monitor_output_ingest.json — so "escalate stuck samplers with kill()" can produce zero data instead of partial data

[milindsrivastava1997]

Hard violation #2 (unconditional timeout=600 + force-kill escalation hitting experiment_run_e2e.py): rather than threading execution_mode/experiment_output_dir scoping into the e2e.py call (which still leaves a "kill after N minutes" semantic that didn't exist before this PR), we removed the force-kill escalation from wait_for_remote_monitor_to_finish() entirely. It's back to its original pre-PR behavior — poll forever, no timeout, no kill — just with the polling now going through the new is_remote_monitor_running() helper. This resolves the issue at the source instead of papering over it with scoping, and doesn't introduce a new "wait forever" → "kill after 10 min" behavior change for callers outside this PR's scope.

Hard violation #1 (SIGTERM/SIGINT handler swallowing): Fix: restore SIG_DFL for SIGTERM/SIGINT right after the ingest loop exits, so a later signal actually terminates the process again.

[milindsrivastava1997]

Removed force kill of remote_monitor. Reverted back to behavior where we wait for remote_monitor to stop gracefully.