Skip to content

feat: update workspace deadline when template policy changes #8964

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

Merged
merged 6 commits into from
Aug 14, 2023

Conversation

deansheather
Copy link
Member

Updates the max deadline for the latest build (if it's the "start" transition) for each workspace using a template when the template changes it's policy.

Recalculate the max deadline using the job's completed_at time as "now" and apply it to the workspace build. If it's before now() + 2h then set the max deadline to now() + 2h. If the existing deadline exceeds the new max deadline, set it to the max deadline.

This only happens when the template policy changes, not when the user changes their quiet hours (to avoid gaming the system, they will need to restart workspaces for the new quiet hours to apply).

Closes #8837

@deansheather deansheather requested a review from Emyrk August 11, 2023 13:24
@deansheather deansheather marked this pull request as ready for review August 11, 2023 13:24
Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

It all looks fine, just wondering if this is going to become a slow function. Can we instrument it or trace it in some way that if it gets slow we can see it?

Comment on lines +144 to +149
if s.UseRestartRequirement.Load() {
err = s.updateWorkspaceBuilds(ctx, db, template)
if err != nil {
return xerrors.Errorf("update workspace builds: %w", err)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

How long can this take if there are a lot of workspaces?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a few queries for each workspace, multiple selects and then an update. I don't think it will take very long, even with thousands of workspaces this should take less than a few seconds. I'll add instrumentation though

@deansheather deansheather requested a review from Emyrk August 14, 2023 20:40
Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

I will leave the approve. Instrumentation would be the only thing I would add 👍

@deansheather deansheather enabled auto-merge (squash) August 14, 2023 21:03
@deansheather deansheather merged commit 47b8bf6 into main Aug 14, 2023
@deansheather deansheather deleted the dean/update-workspace-deadline branch August 14, 2023 21:16
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

autostop requirement: automatically update all workspaces when policy is updated
2 participants