- Notifications
You must be signed in to change notification settings - Fork1k
fix: prevent activity bump for prebuilt workspaces#19263
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
also missing tests here
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.
Can you check theEXPLAIN
output before and after?
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.
The query already filters byworkspace_id
, so theowner_id
predicate is evaluated only after fetching that single row. This means there should be no measurable performance difference.
EXPLAIN
output:
- (before) without prebuild filter:https://explain.dalibo.com/plan/39481703a318fe5b
- (after) with prebuild filter:https://explain.dalibo.com/plan/703fb838d76g86d8
ifallowed { | ||
nextAutostart=next | ||
// Prebuilds are not subject to activity-based deadline bumps | ||
if!workspace.IsPrebuild() { |
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.
suggestion: invert the logic and continue / return early
ssncferreiraAug 19, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Initially, that was my approach, but this function seems to do a lot of things:
- Update agent stats
- Update prometheus metrics (for observability)
- Activity bump (for lifecycle management)
- Bump workspace
last_used_at
(for usage tracking) - Send
stats_update
notify event (AFAIU this is related with real-time updates on the UI regarding the workspace).
Therefore, I kept all these updates except the activity bump since it was the only related with lifecycle parameters. For the case of prebuilds I believe it should be all or nothing, either this approach or we check if it is a prebuild in the beginning of the function and return early. Not sure what is the workflow that makes more sense here for prebuilds.
## DescriptionThis PR ensures that prebuilt workspaces are properly excluded from thelifecycle executor and treated as a separate class of workspaces, fullymanaged by the prebuild reconciliation loop.It introduces two lifecycle guarantees:* When a prebuilt workspace is created (i.e., when the workspace buildcompletes), all lifecycle-related fields are unset, ensuring theworkspace does not participate in TTL, autostop, autostart, dormancy, orauto-deletion logic.* When a prebuilt workspace is claimed, it transitions into a regularuser workspace. At this point, all lifecycle fields are correctlypopulated according to template-level configurations, allowing theworkspace to be managed by the lifecycle executor as expected.## Changes* Prebuilt workspaces now have all lifecycle-relevant fields unsetduring creation* When a prebuild is claimed:* Lifecycle fields are set based on template and workspace levelconfigurations. This ensures a clean transition into the standardworkspace lifecycle flow.* Updated lifecycle-related SQL update queries to explicitly excludeprebuilt workspaces.## Relates Related issue:#18898To reduce the scope of this PR and make the review process moremanageable, the original implementation has been split into thefollowing focused PRs:*#19259*#19263*#19264*#19265These PRs should be considered in conjunction with this one tounderstand the complete set of lifecycle separation changes for prebuiltworkspaces.
…vity-bump-prebuilds
…vity-bump-prebuilds
…vity-bump-prebuilds
…vity-bump-prebuilds
560cf84
intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Description
This PR ensures that activity-based deadline extensions ("activity bumping") are not applied to prebuilt workspaces. Prebuilds are managed by the reconciliation loop and must not have
deadline
ormax_deadline
values set or extended, as they are not part of the regular lifecycle executor path.Changes
ActivityBumpWorkspace
SQL query to discard prebuilt workspacesRelated with: