Skip to content

feat!: bump workspace activity by 1 hour #10704

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 17 commits into from
Nov 15, 2023
Merged

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Nov 14, 2023

What this does

Workspace activity will bump the workspace build deadline by 1 hour.

If the activity bump will cross an autostart threshold, then the bump amount will be the TTL instead. This is so if a workspace is active through the night (likely from an idle terminal), when the next morning comes, the workspace deadline will be bumped as if it autostarted that morning.

If max_deadline is set, it will not be bumped and honored from the original start.

A suggestion would be to max max_deadline < 24 hours to make sure workspaces are restarted every night to avoid this entirely.

Closes https://github.com/coder/customers/issues/318

Will bump by ttl if crosses an autostart threshold
@Emyrk Emyrk marked this pull request as ready for review November 15, 2023 02:27
@Emyrk Emyrk requested a review from johnstcn November 15, 2023 02:27
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

I'm concerned about the additional complexity in the datbase query to perform the bump.

I'm also concerned that this behavioural change may be unexpected, so we should definitely document this new behaviour as well and sure it's called out in the release notes.

Personally, I would be more in favour of just unconditionally bumping the deadline 1 hour each time.

However, I'm not sure if this 'wrap-around' behaviour was a requested feature.

So approving with the above caveats.

Comment on lines 26 to 27
-- relevant. If a past autostart is being passed in,
-- that is a mistake by the caller.
Copy link
Member

Choose a reason for hiding this comment

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

review: not necessarily, this can happen due to a failed get from the template schedule store.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then it would be the zero time. Let me clarify that I mean passing in a timestamp that is in the past, but above the zero time.

@Emyrk
Copy link
Member Author

Emyrk commented Nov 15, 2023

The wrap around behavior was the reason the ttl bump happened originally. I am unsure if it was reported by customers, but it was happening to @ammario enough for the original change.

I feel putting it back to just the simple 1hr bump would land us where we are again right now.

I agree it is a more complex query, which is unfortunate. I just see this as the easier solution if you are trying to solve the wrap around problem and keep activity bumps short (1h).

@Emyrk Emyrk changed the title feat: workspace activity bump by 1 hour feat!: bump workspace activity by 1 hour Nov 15, 2023
@github-actions github-actions bot added the release/breaking This label is applied to PRs to detect breaking changes as part of the release process label Nov 15, 2023
@Emyrk Emyrk merged commit 290180b into main Nov 15, 2023
@Emyrk Emyrk deleted the stevenmasley/activity_bumping branch November 15, 2023 15:42
@github-actions github-actions bot locked and limited conversation to collaborators Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release/breaking This label is applied to PRs to detect breaking changes as part of the release process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants