feat: add queue_position and queue_size to provisioner jobs #8074
Conversation
This should help with monitoring for large-scale deployments. We can add an indicator to the UI for the queue position of workspaces!
|
Does this take into account tagged provisioners and jobs? Two examples come to mind: Case 1) A user starts a build that requires provisioners with a specific tag ( Case 2) A user starts a build that requires provisioners with a specific tag (`environment: on-prem). There is a queue of 100 builds and 20 provisioners, but all on-prem provisioners are offline. I don't think either of these needs to be addressed now, we have an internal RFC that outlines some of these problems. I was mostly curious. There is also no way for a tagged provisioner to reject jobs (#6442), which is also mentioned there. FWIW, I don't think it's a bad UX for "position in queue" to ignore tags, as long as we have to surface some error when no provisions are available with a given tag. For example, when you are looking at your spot in line on a restaurant's app, it is common for that queue position to be a "worse case" and for you to quickly jump spots in line based on your party size, etc. |
|
@bpmct it doesn't in the present form, but can with extreme ease (we just join based on tags). We can improve this in the future once the tagging system and list of provisioners becomes visible in the UI. |
| job, err := api.Database.GetProvisionerJobByID(ctx, templateVersion.JobID) | ||
| if err != nil { | ||
| jobs, err := api.Database.GetProvisionerJobsByIDsWithQueuePosition(ctx, []uuid.UUID{templateVersion.JobID}) | ||
| if err != nil || len(jobs) == 0 { |
There was a problem hiding this comment.
I'm not sure I really like needing to shoehorn this new function to fulfil both of the old queries. Do we really need to return the queue position in all of these routes? What if the frontend only fetched them when it knows it's waiting on a build?
There was a problem hiding this comment.
I think the schema should stay consistent rather than occasionally being there. Unless we observe a performance problem of course.
There was a problem hiding this comment.
Should we remove the old queries then? Would be nice to have a :one and :many query as well so you don't have to || len(jobs) == 0 all the time. That seems like a pretty easy footgun
There was a problem hiding this comment.
I did that initially, but figured that was a mistake because we frequently query just to collect perms, where it's a complete waste to get the queue pos.
There was a problem hiding this comment.
bump
Would be nice to have a :one and :many query as well so you don't have to || len(jobs) == 0 all the time. That seems like a pretty easy footgun
There was a problem hiding this comment.
Ahh, I think the opposite, actually. Because the query itself has many joins and such, I think it'd be a mistake to duplicate that.
There was a problem hiding this comment.
sad that sqlc doesn't have better templating for this
Co-authored-by: Colin Adler <colin1adler@gmail.com>
This should help with monitoring for large-scale deployments.
We can add an indicator to the UI for the queue position of workspaces!