Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[queue time] Add Job Queue Time hook #6417

Closed
wants to merge 11 commits into from
Closed

[queue time] Add Job Queue Time hook #6417

wants to merge 11 commits into from

Conversation

yangw-dev
Copy link
Contributor

@yangw-dev yangw-dev commented Mar 15, 2025

Description

Add git work to insert new queue time data into s3 every 15 minutes,

the new data includes:
{ queue_s, repo, workflow_name , job_name, machine_type, time}

Pending PR for permission:
https://github.com/pytorch-labs/pytorch-gha-infra/pull/627

Pending PR for db schema:
#6418

Design Doc:
https://docs.google.com/document/d/1OiPv-ku_NvMgvnaMIbtnIEHTixJY28CBaKeMKJtglEk/edit?tab=t.0#heading=h.87bg49h503n7

Details

  • refactor query queued_jobs to have workflow_name and job_name field
  • add data modifier func to provide customize pre-rendering process for input data in UI component PanelTable
  • add workflow UpdateJobQueueTimesDataset to upload data to s3

working result in s3

https://us-east-1.console.aws.amazon.com/s3/object/ossci-raw-job-status?region=us-east-1&bucketType=general&prefix=job_queue_times_historical/pytorch/pytorch/1742001203.txt

Copy link

vercel bot commented Mar 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
torchci ✅ Ready (Inspect) Visit Preview Mar 15, 2025 1:23am

@facebook-github-bot facebook-github-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 Mar 15, 2025
@yangw-dev yangw-dev marked this pull request as ready for review March 15, 2025 00:25
@yangw-dev yangw-dev requested review from seemethere and removed request for ZainRizvi March 15, 2025 00:31
@yangw-dev yangw-dev changed the title typo [queue time] Add Job Queue Time hook Mar 15, 2025
@yangw-dev yangw-dev requested a review from clee2000 March 15, 2025 00:33
id: aws_creds
uses: aws-actions/configure-aws-credentials@v3
with:
role-to-assume: arn:aws:iam::308535385114:role/gha_workflow_update_queue_times
Copy link
Contributor Author

@yangw-dev yangw-dev Mar 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I used the same aws role as the update-queue-times.yml.
@jeanschmidt

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't do that, each role for each UC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sonds good

@jeanschmidt
Copy link
Contributor

I don't like that we're using test-infra ci for those kind of jobs. I believe we should stop this bad engineering standard. Maybe this should be a AWS lambda?

@@ -0,0 +1,26 @@
name: Update job queue times dataset
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is a good standard to use GHA CI for chron jobs, I believe we should be using lambdas or other things for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious, is there reason why the old one exists?

Copy link
Contributor

@jeanschmidt jeanschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to think better ways of doing this.

@yangw-dev yangw-dev removed the request for review from seemethere March 17, 2025 17:57
@yangw-dev yangw-dev closed this Mar 18, 2025
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.

3 participants