- Notifications
You must be signed in to change notification settings - Fork920
feat: implement scheduling mechanism for prebuilds#18126
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
feat: implement scheduling mechanism for prebuilds#18126
Conversation
606894f
toff0e813
Compare9af5e02
tobcfbb04
Compare3a25178
toe0d1de7
Compare80e52ee
to47cb6fc
CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
@@ -0,0 +1,12 @@ | |||
-- Add autoscaling_timezone column to template_version_presets table | |||
ALTER TABLE template_version_presets | |||
ADD COLUMN autoscaling_timezone TEXT DEFAULT 'UTC' NOT NULL; |
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.
Why do we need a default here, if the provider is not defining a default?
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.
It'snot null
, so it requires default. You're suggestingDEFAULT ''
?
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.
it was defensive mechanism to make sure we don't fail here ontime.LoadLocation(p.Preset.SchedulingTimezone)
:
func (pPresetSnapshot)CalculateDesiredInstances(at time.Time)int32 {iflen(p.PrebuildSchedules)==0 {// If no schedules are defined, fall back to the default desired instance countreturnp.Preset.DesiredInstances.Int32}// Validate that the provided timezone is valid_,err:=time.LoadLocation(p.Preset.SchedulingTimezone)iferr!=nil {p.logger.Error(context.Background(),"invalid timezone in prebuild scheduling configuration",slog.F("preset_id",p.Preset.ID),slog.F("timezone",p.Preset.SchedulingTimezone),slog.Error(err))// If timezone is invalid, fall back to the default desired instance countreturnp.Preset.DesiredInstances.Int32}}
It shouldn't happen, because iflen(p.PrebuildSchedules) > 0
-SchedulingTimezone
should be set and valid according to validation rules intf-provider-coder
. Even if this happens we should return default - so we should be safe.
But I addedDEFAULT 'UTC'
as another layer of defense.
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.
Final decision: changed to''
instead ofUTC
coderd/database/migrations/000335_add_autoscaling_to_presets.up.sql OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
The "continuous time" interpretation of the crontab makes it, I guess, relatively simple to specify spans that start and end on the hour, but consider attempting to program a span like 7:45am - 9:10am. You'd need 3 spans
Are we just kind of assuming that operators likely only want hourly precision? |
Co-authored-by: Danny Kopping <danny@coder.com>
…toscaling-mechanism
Co-authored-by: Danny Kopping <danny@coder.com>
Minutes must always be If we allow specific minute ranges (e.g., Therefore, the minimum supported granularity is one hour. There was a long discussion, and majority voted for this approach. Alternative approaches are described here:https://www.notion.so/coderhq/Implement-autoscaling-mechanism-201d579be5928054837dceb8358bfed3?source=copy_link#207d579be5928059b5ded1778841fba2 |
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.
Excellent work@evgeniy-scherbina!
I suspectcoder/terraform-provider-coder#408 might prompt a naming change, but I'm happy to approve as-is.
Uh oh!
There was an error while loading.Please reload this page.
RunningPrebuilds []database.GetRunningPrebuiltWorkspacesRow | ||
PrebuildsInProgress []database.CountInProgressPrebuildsRow | ||
Backoffs []database.GetPresetsBackoffRow | ||
HardLimitedPresetsMap map[uuid.UUID]database.GetPresetsAtFailureLimitRow | ||
clock quartz.Clock |
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.
I don't mind either approach, but let's pick one 👍
Uh oh!
There was an error while loading.Please reload this page.
…toscaling-mechanism
0f6ca55
intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Closescoder/internal#312
Depends oncoder/terraform-provider-coder#408
This PR adds support for defining anautoscaling block for prebuilds, allowing number of desired instances to scale dynamically based on a schedule.
Example usage:
Behavior
schedule
blocks perprebuilds
block are supported.prebuilds.instances
) is used as a fallback.Why
This feature allows prebuild instance capacity to adapt to predictable usage patterns, such as:
Cron specification
The cron specification is interpreted as acontinuous time range.
For example, the expression:
is intended to represent a continuous range from09:00 to 18:59, Monday through Friday.
However, due to minor implementation imprecision, it is currently interpreted as a range from08:59:00 to 18:58:59, Monday through Friday.
This slight discrepancy arises because the evaluation is based on whether a specificpoint in time falls within the range, using the
github.com/coder/coder/v2/coderd/schedule/cron
library, which performs per-minute matching rather than strict range evaluation.