Skip to content

Feat; status page queries #2217

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Jamesbarford
Copy link
Contributor

This should have no performance bottle necks; Both in_progress and completed are obtained in 2 queries.

Roughly we have;

completed_requests: [ (request, duration, Vec<error>) ]

in_progress: [ (request, Vec<Job>) ]

Then in the website we'd compose the queue by calling the method from the job_queue module.

Annoyingly we don't have any database deserialisation so I've had to do it manually. The test probably gives the best example of why this approach is handy. I've tried to push all of the oddities into the postgres.rs file so nothing leaks out.

@Jamesbarford Jamesbarford force-pushed the feat/status-page-queries branch from 0a29b2a to b26fd62 Compare August 7, 2025 10:16
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! It's a bit weird to encode the knowledge of status page data in the database crate, but I guess that it is better than splitting it into multiple methods on the pool, as we want to get highly specialized results here, and ideally in an optimal manner. So fine by me.

ORDER BY
completed_at
DESC LIMIT {max_completed_requests}
), jobs AS (
Copy link
Member

Choose a reason for hiding this comment

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

I thought about this a bit more and I think that we should probably just compute the request computation duration at completion time, and then just store it as a field in the benchmark request table. It will allow us to avoid doing this job query everytime we load the page, and it makes the duration immutable. If we load the jobs on every status page load, then if the jobs disappear, the duration can change (should be quite rare), but more importantly if we backfill the request in the meantime, then suddenly the duration of the request would jump to some ludicrously long duration, because we'd get a new recent completed job, while the old completed jobs could be e.g. 1-2 days old.

Computing the duration once at the time the request is completed (ideally in the mark_request_as_completed query) would avoid these, so I would prefer doing that.

), artifacts AS (
SELECT
artifact.id,
\"name\"
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need to be quoted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

name is a keyword in postgres to use keywords as column names you need to double quote them. However name is un-reserved so I can remove it. The editor (dbeaver) I was writing this SQL in automatically did it.

), errors AS (
SELECT
artifacts.name AS tag,
ARRAY_AGG(error) AS errors
Copy link
Member

Choose a reason for hiding this comment

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

Huh, interesting. The error count should be low, so normally I would just do a join and call it a day, but if this works, then why not.

Copy link
Contributor Author

@Jamesbarford Jamesbarford Aug 8, 2025

Choose a reason for hiding this comment

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

Something to note on the error table is that it needs a unique "krate", aid pairing. Which if we are using for arbitrary error collection may mean we need think about this

Copy link
Member

Choose a reason for hiding this comment

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

Well, it's called "krate", but it's really just an arbitrary string, so we can put there "job-queue-error" or whatever.

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