- Notifications
You must be signed in to change notification settings - Fork1.1k
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
606894f toff0e813Compare9af5e02 tobcfbb04Compare3a25178 toe0d1de7Compare80e52ee to47cb6fcCompareUh 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 | |||
| ALTERTABLE template_version_presets | |||
| ADD COLUMN autoscaling_timezoneTEXT 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.
spikecurtis commentedJun 13, 2025
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>
evgeniy-scherbina commentedJun 17, 2025
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 |
dannykopping left a comment
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.
| PrebuildsInProgress []database.CountInProgressPrebuildsRow | ||
| Backoffs []database.GetPresetsBackoffRow | ||
| HardLimitedPresetsMapmap[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
scheduleblocks perprebuildsblock 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/cronlibrary, which performs per-minute matching rather than strict range evaluation.