Remove configurable serverless min ready#110
Conversation
a277a48 to
a3e9d74
Compare
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
8f4d13d to
c25f2b4
Compare
techulus-agent
left a comment
There was a problem hiding this comment.
Review — Remove configurable serverless min ready
Reviewed the diff (base feat/agent-disk-cleanup). Clean, well-scoped removal — the "always emit minReadyReplicas: 1, keep the DB column for compat" approach is handled consistently across every read path. Ran tests/service-config.test.ts + tests/expected-state.test.ts locally: 23/23 pass.
Verified
- No dead imports: the removed
updateServiceConfigclamping block dropped the last use ofMath.max(1, totalReplicas)and theLEAST(...)SQL, butsqlandandare still used elsewhere inactions/projects.ts, so nothing is left dangling.totalReplicaswas removed cleanly. - Read-path normalization is complete:
buildCurrentConfig,normalizeServerlessConfig, andgetCurrentServerlessConfigall now return1, andgetDeployedServerlessConfigroutes throughnormalizeServerlessConfig/getCurrentServerlessConfig— so legacy stored values> 1are normalized on read. The new regression tests (stored7, deployed5→1) correctly exercise this. - Agents never see stale values: the canonical route config in
buildServerlessRoutesFromRowshardcodesminReadyReplicas: 1, anddiffConfigsno longer compares the field, so a legacy DB value can't produce a spurious config diff or leak to a proxy. - UI fully cleaned: the only caller of
updateServiceServerlessSettings(serverless-section.tsx) was updated, the input/state/validation/hasChangeswere all removed, and the grid droppedmd:grid-cols-3→md:grid-cols-2to match the two remaining fields. No leftoverMin Readystrings anywhere incomponents//app/. - Docs (
architecture.mdx,scaling.mdx) updated to match the fixed one-ready-upstream behavior.
Non-blocking notes
lib/agent-status.tsstillSELECTsserverlessMinReadyReplicas(lines 152, 284) and types it (~564), but nothing consumes it for behavior now that the value is fixed. Harmless (compat field intentionally kept), just a candidate to trim in a later cleanup.- Stacked on #108 — base is
feat/agent-disk-cleanup, so merge #108 first (or rebase ontomain) to keep the diff/history clean.
Result: No correctness issues. LGTM 👍
4830467 to
0b618d5
Compare
techulus-agent
left a comment
There was a problem hiding this comment.
Re-review — full removal of minReadyReplicas 👍
This revision is a much cleaner approach than the previous "keep the column, always emit 1" version — it now removes minReadyReplicas end-to-end: DB column, TS/Go types, the agent ServerlessRoute struct field, the gateway wake logic, and all tests/docs. Verified locally: go build ./... clean, agent internal/serverless tests pass, web service-config + expected-state suites pass (22/22).
Verified correct
- Gateway wake logic is a faithful collapse to min-ready = 1. In both
resolveUpstreamsandwaitForReadyUpstreams,len(ready) >= max(1, route.MinReadyReplicas) || (…)becomeslen(ready) > 0. With min-ready = 1,len(ready) >= 1≡len(ready) > 0and already subsumed the OR clause, so behavior is identical for the default case. For legacy services that had min-ready > 1, requests now resume as soon as one upstream is ready — which is exactly the intended new semantics. The blocking/else branch is still reachable whenlen(ready) == 0, so nothing is stranded. - Agent ↔ control-plane rollout is safe in both directions.
encoding/jsonignores unknown fields, so a new agent silently ignores an old control plane still emittingminReadyReplicas; and an old agent receiving the new field-less JSON gets Go's zero value0, which itsmax(1, 0)= 1 handles gracefully. No ordering constraint between web and agent deploys. - Complete removal — zero remaining references to
minReadyReplicas/serverless_min_ready/MinReadyReplicasanywhere in web, agent, docs, or tests. The deadagent-status.tsselects I flagged last round are gone too.
Notes
- Dead code:
hasAlwaysOnUpstream(agent/internal/serverless/gateway.go:912) is now defined but never called. Go still compiles, butstaticcheck(U1000) and reviewers will flag it — worth deleting in this PR. - DB column drop: since the repo uses
drizzle-kit push(no versioned migrations), removing the column fromschema.tsis the right mechanism —db:pushwill dropserverless_min_ready_replicasat deploy. Just confirm the deploy pipeline runs push with confirmation/--forcesince this is a destructive change (the drop itself is intended — the value is being abandoned).
Result: No correctness issues; the two notes are cleanup/ops, not blockers. LGTM 👍
0b618d5 to
f329e07
Compare
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Summary
Removes
Min Ready/minReadyReplicasentirely from serverless configuration and routing. The wake gateway now resumes queued requests when the first upstream is ready.Why
minReadyReplicaswas being treated like a cluster-level replica setting, but serverless wake behavior is proxy-local. Keeping it around as a fixed value still left a ghost configuration knob in the control-plane/agent contract, so this removes the field instead of pinning it to one.Changes
Min Readyinput and action payload field.minReadyReplicasfrom current/deployed serverless config, expected state, and the agent serverless route payload.serverless_min_ready_replicasschema column.serverlessMinReadyReplicasselect/type plumbing from agent status handling.Validation
pnpm test -- tests/service-config.test.ts tests/expected-state.test.tspnpm exec tsc --noEmitpnpm testgo test ./...fromagentpnpm exec biome check lib/agent-status.ts actions/projects.ts components/service/details/serverless-section.tsx lib/agent/expected-state.ts lib/service-config.ts tests/expected-state.test.ts tests/service-config.test.tsgit diff --checkrg -n "minReadyReplicas|MinReadyReplicas|serverlessMinReadyReplicas|serverless_min_ready_replicas" . -g '!web/pnpm-lock.yaml'Biome exits successfully but still reports existing warnings in
lib/agent/expected-state.tsandtests/expected-state.test.ts.