diff --git a/src/sim/behaviour-control.autoscaler.test.ts b/src/sim/behaviour-control.autoscaler.test.ts new file mode 100644 index 0000000..327140a --- /dev/null +++ b/src/sim/behaviour-control.autoscaler.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it } from 'vitest'; +import { Engine } from './engine'; +import { makeNode } from './presets'; +import type { NodeStats, Topology } from './types'; + +/* + * None of the autoscaler's knobs is among the nine config numbers + * `isTopology` checks, so a shared link, a `.breakscale` file and a restored + * session can all hand the controller a value that is not a number. What it + * does with one is a property of the controller. + * + * The reading that matters is not the knob, it is whether the fleet still + * grows: a controller that quietly stops controlling looks exactly like a + * design that did not need to scale. + */ + +function topology(patch: Record): Topology { + const client = { ...makeNode('client', 0, 0), id: 'client' }; + client.config = { ...client.config, rps: 400 }; + const svc = { ...makeNode('service', 200, 0), id: 'svc' }; + svc.config = { + ...svc.config, + capacity: 2, + serviceMs: 40, + instances: 1, + } as typeof svc.config; + const auto = { ...makeNode('autoscaler', 200, 160), id: 'auto' }; + auto.config = { ...auto.config, ...patch } as typeof auto.config; + return { + nodes: [client, svc, auto], + edges: [ + { id: 'e1', from: 'client', to: 'svc', weight: 1 }, + { id: 'e2', from: 'auto', to: 'svc', weight: 1, control: true }, + ], + }; +} + +function run(patch: Record) { + const engine = new Engine(topology(patch), 7); + // Thirty simulated seconds: several cooldowns, so the controller has had + // every chance to act. + for (let i = 0; i < 1800; i += 1) engine.advance(1000 / 60); + const snapshot = engine.snapshot(); + return { + auto: snapshot.nodes['auto'] as NodeStats, + svc: snapshot.nodes['svc'] as NodeStats, + }; +} + +const KNOBS = [ + 'targetUtil', + 'minCapacity', + 'maxCapacity', + 'scaleStepPct', + 'cooldownMs', + 'warmupMs', +] as const; + +describe('an autoscaler given a knob that is not a number', () => { + const baseline = run({}); + + it('scales the fleet in the baseline design', () => { + // Anchors the rest: this load genuinely needs more than one instance. + expect(baseline.svc.instances).toBeGreaterThan(5); + expect(baseline.svc.totalCompleted).toBeGreaterThan(5000); + }); + + for (const key of KNOBS) { + it(`still scales when ${key} is NaN`, () => { + // Before this change targetUtil, scaleStepPct and warmupMs each left + // the fleet at one instance and the design served 1520 requests where + // the baseline serves 6756. + const { svc } = run({ [key]: Number.NaN }); + expect(svc.instances).toBeGreaterThan(5); + expect(svc.totalCompleted).toBeGreaterThan(4000); + }); + } + + // Where the module's documented fallback is also what the node carries, + // a NaN is indistinguishable from the value it replaced -- which is the + // sharpest available statement that the guard changed nothing real. + for (const key of ['targetUtil', 'minCapacity', 'scaleStepPct'] as const) { + it(`is identical to the baseline when ${key} is NaN`, () => { + const { svc } = run({ [key]: Number.NaN }); + expect(svc.instances).toBe(baseline.svc.instances); + expect(svc.totalCompleted).toBe(baseline.svc.totalCompleted); + }); + } + + it('publishes a setpoint that is a number', () => { + const { auto } = run({ targetUtil: Number.NaN }); + expect(Number.isNaN(auto.setpoint as number)).toBe(false); + }); + + it('publishes a target instance count that is a number', () => { + const { auto } = run({ scaleStepPct: Number.NaN }); + expect(Number.isNaN(auto.targetInstances as number)).toBe(false); + }); + + it('leaves a design the editor can produce exactly where it was', () => { + // The knobs a reader sets are read the same way they always were, so a + // design that names all six is untouched by the guard. + const explicit = run({ + targetUtil: 0.7, + minCapacity: 1, + maxCapacity: 12, + cooldownMs: 3000, + scaleStepPct: 0.5, + warmupMs: 4000, + }); + expect(explicit.svc.instances).toBe(baseline.svc.instances); + expect(explicit.svc.totalCompleted).toBe(baseline.svc.totalCompleted); + }); +}); diff --git a/src/sim/behaviour-control.ts b/src/sim/behaviour-control.ts index 973713c..3e1723b 100644 --- a/src/sim/behaviour-control.ts +++ b/src/sim/behaviour-control.ts @@ -54,6 +54,21 @@ const DEFAULT_COOLDOWN_MS = 5000; const DEFAULT_STEP_PCT = 0.5; const DEFAULT_WARMUP_MS = 0; +/** + * One optional knob, or its fallback. + * + * `??` delivers the promise above for an UNSET field and not for one set to + * something that is not a number, and none of these knobs is among the nine + * config numbers `isTopology` checks -- so a shared link, a `.breakscale` + * file and a restored session can all hand the controller a NaN. NaN then + * survives `Math.floor`, `Math.max` and `clamp01` alike, because it fails + * every comparison those are written from, and the controller stops + * controlling. + */ +function knob(value: number | undefined, fallback: number): number { + return Number.isFinite(value) ? (value as number) : fallback; +} + interface AutoscalerState { /** Simulated time of the last decision; -Infinity means "never decided". */ lastDecisionMs: number; @@ -185,7 +200,7 @@ const autoscaler: ComponentBehaviour = { // ...and starts a fresh observation window, because the new node's // smoothed utilisation is not a signal yet. st.observeUntilMs = - ctx.now + Math.max(0, state.config.cooldownMs ?? DEFAULT_COOLDOWN_MS); + ctx.now + Math.max(0, knob(state.config.cooldownMs, DEFAULT_COOLDOWN_MS)); } if (watched === '') return; @@ -218,7 +233,7 @@ const autoscaler: ComponentBehaviour = { if (st.warmupDueMs >= 0) return; const cfg = state.config; - const cooldown = Math.max(0, cfg.cooldownMs ?? DEFAULT_COOLDOWN_MS); + const cooldown = Math.max(0, knob(cfg.cooldownMs, DEFAULT_COOLDOWN_MS)); // Hold off until the watched node's utilisation is a real measurement // rather than an average still climbing out of its initial zero. @@ -241,13 +256,16 @@ const autoscaler: ComponentBehaviour = { // the unit they bound is now INSTANCES -- the fleet size, not the thread // count. For every topology written before instances existed the two // readings coincide, because those nodes run exactly one instance. - const minInst = Math.max(1, Math.floor(cfg.minCapacity ?? DEFAULT_MIN_INSTANCES)); + const minInst = Math.max( + 1, + Math.floor(knob(cfg.minCapacity, DEFAULT_MIN_INSTANCES)), + ); const maxInst = Math.max( minInst, - Math.floor(cfg.maxCapacity ?? DEFAULT_MAX_INSTANCES), + Math.floor(knob(cfg.maxCapacity, DEFAULT_MAX_INSTANCES)), ); - const target = clamp01(cfg.targetUtil ?? DEFAULT_TARGET_UTIL); - const step = Math.max(0.01, cfg.scaleStepPct ?? DEFAULT_STEP_PCT); + const target = clamp01(knob(cfg.targetUtil, DEFAULT_TARGET_UTIL)); + const step = Math.max(0.01, knob(cfg.scaleStepPct, DEFAULT_STEP_PCT)); // An instance count is integral, so a step must move at least one machine: // a small percentage of a small fleet would otherwise round to a permanent // no-op and the controller would silently do nothing forever. @@ -300,7 +318,7 @@ const autoscaler: ComponentBehaviour = { return; } - const warmup = Math.max(0, cfg.warmupMs ?? DEFAULT_WARMUP_MS); + const warmup = Math.max(0, knob(cfg.warmupMs, DEFAULT_WARMUP_MS)); if (warmup === 0) { st.targetInstances = want; ctx.setScale(watched, want); @@ -338,12 +356,12 @@ const autoscaler: ComponentBehaviour = { stats.pendingInstances = scaling ? Math.max(0, wanted - live) : 0; stats.scaling = scaling; stats.watchedUtil = st.watchedId ? (ctx.utilizationOf(st.watchedId) ?? 0) : 0; - stats.setpoint = clamp01(state.config.targetUtil ?? DEFAULT_TARGET_UTIL); + stats.setpoint = clamp01(knob(state.config.targetUtil, DEFAULT_TARGET_UTIL)); // Which of the three waits it is in, and how much of it is left. Resolved // in the same order onTick() applies them, so the label never claims the // controller is free to act when the next tick will find it blocked. - const cooldown = Math.max(0, state.config.cooldownMs ?? DEFAULT_COOLDOWN_MS); + const cooldown = Math.max(0, knob(state.config.cooldownMs, DEFAULT_COOLDOWN_MS)); if (scaling) { stats.scalePhase = 'warming'; stats.phaseRemainingMs = Math.max(0, st.warmupDueMs - ctx.now);