Skip to content

[HUD] Backfill ExecuTorch workflow jobs in preparation for including ET on HUD queueing - #8643

Draft
georgehong wants to merge 1 commit into
mainfrom
hud-executorch-job-backfill
Draft

[HUD] Backfill ExecuTorch workflow jobs in preparation for including ET on HUD queueing #8643
georgehong wants to merge 1 commit into
mainfrom
hud-executorch-job-backfill

Conversation

@georgehong

Copy link
Copy Markdown
Contributor

Include PyTorch and ExecuTorch in stale workflow-job repair while preserving the existing per-pass request limit through fair repository interleaving. Add a bounded dry-run path and avoid treating transient GitHub errors as missing jobs. This is intended to be the prerequisite change for including ET workflow/queueing results in HUD.

Dry-Run:

DOTENV_CONFIG_PATH=.env.local node -r dotenv/config scripts/backfillJobs.mjs --dry-run --limit 10

After land, we can track things here: https://github.com/pytorch/test-infra/actions/workflows/backfill-workflow-job.yml

Include PyTorch and ExecuTorch in stale workflow-job repair while preserving the existing per-pass request limit through fair repository interleaving. Add a bounded dry-run path and avoid treating transient GitHub errors as missing jobs.
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 26, 2026
@vercel

vercel Bot commented Aug 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
torchci Ignored Ignored Aug 26, 2026 10:30pm

Request Review

@georgehong
georgehong marked this pull request as ready for review August 26, 2026 22:33
@huydhn

huydhn commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

@claude Check if the SQL changes are correct and works with different repos

@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown

Claude finished @huydhn's task in 2m 35s —— View job


SQL review: multi-repo backfill correctness

  • Read the PR diff and understand the SQL changes
  • Check correctness of the SQL for multi-repo backfill
  • Verify per-repo fairness / interleaving logic
  • Post review feedback

I checked the two rewritten queries (jobsWithNoConclusion and queuedJobs) plus the parameterized incomplete-lookup query. Short answer: the SQL is correct and does work generically across the repos in BACKFILL_REPOSITORIES. A few notes below.

✅ What's correct

1. Multi-repo filtering is consistent. Both the pending_jobs CTE (j.repository_full_name in {repositories}) and the outer workflow_run filter (w.repository.'full_name' in {repositories}) now use the parameterized array instead of the old hardcoded w.repository.'name' = 'pytorch'. Filtering on full_name (owner/repo) is the right choice — name = 'pytorch' alone would have been ambiguous, and it lets you add any repo to the list with no query changes. I confirmed repository_full_name and started_at are real columns on workflow_job (used the same way in hud_query, tts_duration_historical, etc.).

2. Fair interleaving is implemented correctly.

row_number() OVER (PARTITION BY w.repository.'full_name' ORDER BY j.started_at ASC) as repository_rank
...
ORDER BY repository_rank ASC, j.started_at ASC
LIMIT {backfillLimit}

This is genuine round-robin: it takes each repo's oldest pending job (rank 1) first, then each repo's 2nd-oldest, etc., before applying the global LIMIT. So the per-pass limit is shared but no repo starves. And it degrades gracefully — if a small repo (say ET with 3 pending jobs) runs out at rank 3, the remaining slots naturally fall through to the larger repo. Works for any N repos.

3. Moving LIMIT out of the CTE is actually more correct than before. In the old no-conclusion query the LIMIT 200 lived inside pending_jobs, which was then referenced twice (the join and w.id in (select run_id from pending_jobs)). With the limit applied only at the outer query, the CTE is now internally consistent between those two references. Correctness win, not just a refactor.

4. Parameterization fixes a latent injection/escaping bug. The incomplete-lookup query moved from string interpolation

`... j.dynamoKey = '${dynamo_key}' and j.id = ${id}`

to bound params ({dynamoKey: String}, {id: UInt64}). Good — this was the one spot that interpolated untrusted-ish values directly.

⚠️ Things worth confirming

1. The unclassified-logs query is still hardcoded to pytorch/pytorch. The third query (unclassifiedJobs, backfillJobs.mjs:373) still has:

and w.head_repository. 'full_name' = 'pytorch/pytorch'

So ExecuTorch failed/cancelled jobs will not get their logs backfilled/classified, even though the first two passes now cover ET. If that's intentional for this PR (queueing data first, log classification later) it's fine — but it's an inconsistency worth a comment, otherwise ET log classification silently never happens. Fix this →

2. UInt64 id round-trip (pre-existing, low risk). With output_format_json_quote_64bit_integers: 0, ids come back as JS numbers and are passed back into {id: UInt64}. GitHub job ids are ~10 digits (well under 2^53), so this is safe today; just flagging that if ids ever exceed 2^53 you'd get precision loss. Not introduced by this PR.

3. Tie-breaking in row_number. ORDER BY j.started_at ASC within a partition is non-deterministic on equal started_at values. Harmless for a backfill (order only affects which jobs land in a capped pass), just noting it.

Verdict

The SQL changes are correct and repo-agnostic — the interleaving and shared-limit logic behaves sensibly for both equal-sized and lopsided repo workloads. The only actionable item is deciding whether the unclassified-logs pass should also include ExecuTorch (#1); the rest are non-blocking notes. I couldn't execute the queries against ClickHouse here (no credentials in this environment), so this is a static review.

@huydhn

huydhn commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

This looks relevant and needs to be fixed:

The unclassified-logs query is still hardcoded to pytorch/pytorch

@georgehong
georgehong marked this pull request as draft August 31, 2026 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants