-
Notifications
You must be signed in to change notification settings - Fork 159
Feat; Split request job into benchmark jobs #2207
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
Feat; Split request job into benchmark jobs #2207
Conversation
There was a problem hiding this 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.
…ault_profiles` in the database as opposed to the collector
754f63b
to
21d8d31
Compare
There was a problem hiding this 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.
034d503
to
c32b63a
Compare
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
I'd end there, because I still need to reland #2161 before we move forward there. I will work on that next. |
Adds the database table
job_queue
along with creating aBenchmarkJob
from aBenchmarkRequest
.This does not include enqueuing a parent if there is a missing backend/profile.