Skip to content

Conversation

@Jamesbarford
Copy link
Contributor

Adds the database table job_queue along with creating a BenchmarkJob from a BenchmarkRequest.

This does not include enqueuing a parent if there is a missing backend/profile.

Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, left a few other thoughts.

I'd like to remove BenchmarkJob::create_queued, because it creates the wrong incentives - it shouldn't be usable in "normal" code, because everything should go through the DB, and it ideally shouldn't be usable in tests, because then we test something that doesn't actually happen in production - even the tests should go through the DB if possible. This is a bit different from the benchmark request, where it made sense to create it in code and then insert it into the DB, because we had several variations of the request that could exist. But the job is more specialized, and it doesn't make sense to create it outside of the database.

It would also be nice to add at least some brief doc comments in code on top of the job, just to explain what is it for.

@Jamesbarford Jamesbarford force-pushed the feat/split-jobs-and-enqueue branch from 754f63b to 21d8d31 Compare July 24, 2025 21:23
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Thanks! I tested this locally and noticed that multiple requests were marked as in progress, because the enqueuing loop didn't end after enqueuing the first request. I fixed that and added more logging, which helped me with debugging locally.

Btw you can run local Postgres DB, run the website with the CRON environment variables and it will actually start creating the master/release artifacts and enqueuing jobs, so it can be easily tested.

@Kobzol Kobzol force-pushed the feat/split-jobs-and-enqueue branch from 034d503 to c32b63a Compare July 25, 2025 07:46
@Kobzol
Copy link
Member

Kobzol commented Jul 25, 2025

Before we go for marking requests as complete, we should work on the collector side, as that's the next functionality in the request/job lifecycle. So there should be a new collector command, something similar to BenchNext, which will:

  • Receive the name of the collector
  • Load the collector config from the DB and update the heartbeat (could we actually do that with a single SQL query? :) )
  • Dequeue a job

I'd end there, because I still need to reland #2161 before we move forward there. I will work on that next.

@Kobzol Kobzol enabled auto-merge July 25, 2025 07:48
@Kobzol Kobzol merged commit f33ebea into rust-lang:master Jul 25, 2025
11 checks passed
@Jamesbarford Jamesbarford deleted the feat/split-jobs-and-enqueue branch August 21, 2025 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants