Repository navigation
feat(asap-tools): Alibaba and Google trace replay builders - #919
milindsrivastava1997 wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GLAs2FV3uZpVD7mUb6vUxk make_replay.py reads MSRTMCR tarballs and takes --column (e.g. providerrpc_mcr); empty values become 0. ALIBABA_MAPPING.md documents the mapping.
make_google_replay.py builds a paced-feeder replay table from task_usage CPU rate in the random workloads' label shape. GOOGLE_MAPPING.md and scripts/ record how the trace subset was chosen. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GLAs2FV3uZpVD7mUb6vUxk
milindsrivastava1997
left a comment
milindsrivastava1997
left a comment
There was a problem hiding this comment.
Automated review (Claude Code). The Alibaba scaling, the null users[j] lookup, and the null-CPU weighting are probably the ones to fix before merging.
| by_series = {} | ||
| for offset, *_, label_0, value in rows: # sorted by offset | ||
| by_series.setdefault(label_0, []).append(round(float(value), 6)) | ||
| by_series.setdefault(label_0, []).append(round(float(value or 0), 6)) |
There was a problem hiding this comment.
The PR description says Alibaba values are scaled ×10⁴ because Count-Min top-k rounds weights to integers (#916), but nothing here scales them, and there's no --scale option like the Google builder has. cpu_utilization runs from about 0.003 to 0.997 and paced-feeder replays with scale 1.0, so value.round() as i64 turns nearly every sample into 0 or 1, which makes top-k on this replay meaningless.
| out = sorted( | ||
| (int(r["timestamp"]) - start, *labels[r["msinstanceid"]], r["cpu_utilization"]) | ||
| (int(r["timestamp"]) - start, *labels[r["msinstanceid"]], r[args.column]) | ||
| for r in rows(args.csv) |
There was a problem hiding this comment.
Missing values are handled inconsistently. <out>.csv keeps the raw empty string from r[args.column], while write_table (line 105) turns it into 0, so the CSV and the JSON table no longer agree. Also, seen (line 62) counts a step even when the value is empty, so a pod with gaps still passes the "reports at every step" filter and its gaps are replayed as made-up zeros. Earlier this input crashed on float(''); now it silently corrupts the data. Consider skipping empty values when building seen, or filling them in one place before both outputs are written.
| {"chosen": chosen}, | ||
| ).fetchall() | ||
| ) | ||
| ordered = sorted(chosen, key=lambda j: (users[j], j)) |
There was a problem hiding this comment.
users[j] can fail. arg_max(...) FILTER (WHERE column4 IS NOT NULL) returns NULL for a job whose events all have a null user, which makes this sort raise a TypeError when it compares None with str. A chosen job with no rows in job_events raises a KeyError. Either way the crash happens after the slow DuckDB pass. Suggest users.get(j) or "", or filtering such jobs out before sampling.
| CREATE TABLE cells AS | ||
| SELECT job, task, (start_us // {STEP_US} - {FIRST_STEP})::SMALLINT AS t, | ||
| -- exact DECIMAL sums keep the mean independent of thread order | ||
| sum(cpu::DECIMAL(18, 12) * w)::DOUBLE / sum(w)::DOUBLE AS cpu |
There was a problem hiding this comment.
Rows with a NULL cpu still add their weight to sum(w) but nothing to the numerator, so cells that mix null and non-null rows come out too low. For example, (0.1, 300s) plus (NULL, 300s) gives 0.05 instead of 0.1. A cell where every row is null gives NULL, then NaN, and fails the assertion at line 162. Suggest sum(w) FILTER (WHERE cpu IS NOT NULL) or WHERE cpu IS NOT NULL in the subquery.
| ) | ||
| with open(f"{args.out}/replay_mapping.csv", "w", newline="") as f: | ||
| w = csv.writer(f) | ||
| w.writerow( |
There was a problem hiding this comment.
The column order here (..., google_job_id, task_index, user) doesn't match GOOGLE_MAPPING.md §5 step 7 (..., user, job_id, task_index). Please update one to match the other.
| os.makedirs(args.out, exist_ok=True) | ||
|
|
||
| con = duckdb.connect() | ||
| tmp = tempfile.mkdtemp(dir=args.out, prefix="duckdb-tmp-") |
There was a problem hiding this comment.
The temp dir is created inside args.out and removed only if the run succeeds and the dir ends up empty (line 185). If the run fails partway, or DuckDB leaves spill files behind, a duckdb-tmp-* directory that can be GB-sized stays in the output directory. tempfile.TemporaryDirectory() cleans up on every path. Separately, replay.csv (lines 175-177) is written row by row in Python, 8.64M rows in total; a DuckDB COPY would be much faster.
| t = time.time() | ||
| con.execute( | ||
| f""" | ||
| COPY (SELECT CAST(c1/1000000 AS INTEGER) AS start_s, CAST(c2/1000000 AS INTEGER) AS end_s, |
There was a problem hiding this comment.
In DuckDB, / is float division and CAST(... AS INTEGER) rounds, so start_s is rounded to the nearest second instead of truncated. steps.py then takes start_s // 300, which can put a row in a different step than make_google_replay.py does with start_us // STEP_US. For example, start_us = 2,246,999,600,000 lands in step 7490 here and step 7489 in the builder. That can make the analysis eligibility counts differ from the builder's. Use c1 // 1000000 instead.
| ZIPF_THETA_BOUNDS = (0.0, 5.0) | ||
|
|
||
|
|
||
| def zipf_mle(weights): # copied from asap-tools/dataset-analysis/fit_skew.py |
There was a problem hiding this comment.
zipf_mle is copied from asap-tools/dataset-analysis/fit_skew.py instead of imported, so the two estimators can drift apart. The # copied from ... comment is also the kind of history note AGENTS.md asks us to keep out of code comments.
Builders for the trace replay tables the paced feeder plays back (feeder support landed in #909).
datasets/alibaba/make_replay.py): reads MSRTMCR tarballs as well as plain CSVs, takes--columnfor the value column (e.g.providerrpc_mcr), and treats empty values as 0.ALIBABA_MAPPING.mddocuments how pods map onto the random workloads' labels.datasets/google/make_google_replay.py): builds a replay table from ClusterData 2011task_usageCPU rate in the same 4 × 5 × 10 × 50 label shape.GOOGLE_MAPPING.mdandscripts/record how the trace subset was chosen.Values are scaled (Alibaba ×10⁴, Google ×10⁶) because Count-Min top-k rounds weights to integers (#916).
Generated tables (Google's is 385 MB) are not committed.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GLAs2FV3uZpVD7mUb6vUxk