Repository navigation
Make the ingress wait configurable and add an autoscaling guide - #270
Conversation
SQL_FS_MAX_INGRESS_WAIT_MS sets how long a request or MCP listener waits for an ingress permit before a 503 EWORKER_BUSY. The default rises from 250 ms to 1000 ms: a refused request is told to retry after 1 s, so refusing a burst sooner only sends the client to the back of the line. The worker queue wait keeps its 250 ms default.
AUTOSCALING.md documents /healthz, /readyz and /metrics, every metric the server emits, the signals to scale on in priority order, what more replicas do not fix, measured single-replica capacity, shutdown draining, the two wait limits, and an Azure Container Apps scale rule. CONFIGURATION.md gains SQL_FS_MAX_INGRESS_WAIT_MS and points its autoscaling notes at the new guide.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
All reported issues were addressed across 9 files
You’re at about 90% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
server.close() waited on every open connection before shutdownMcp() ran, so a client holding a GET /mcp listener stream blocked the worker drain until the 60 s force exit. Shutdown now ends standalone listener streams first and leaves in-flight tool calls to finish.
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7dd026948a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0ac855ee2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed
You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
@codex review |
…ait for child close
There was a problem hiding this comment.
All reported issues were addressed
You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed
You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 289a54b603
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…dline at shutdown
PR branches matching the push patterns ran each workflow twice per push, once for push and once for pull_request. The pull_request run still reports every required check.
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Tip: Review your code locally with the cubic CLI to iterate faster.
View guided diff | Turn on auto-fix | Re-trigger cubic
Stacked on #268. Review and merge #268 first. This PR's base is
feat/workers-default-231.What changes
SQL_FS_MAX_INGRESS_WAIT_MS. It sets how long a request or MCP listener waits for ingress capacity before it gets a 503EWORKER_BUSY. Before, the wait was a hard-coded 250 ms. The default stays 250 ms, so nothing changes unless an operator sets it. It is read the same way as the otherSQL_FS_MAX_INGRESS_*settings.docs/AUTOSCALING.md./healthz,/readyzand/metricsendpoints, and how a scraper authenticates to/metrics./metricsemits. The names were captured from a running server's/metricsoutput and checked againstmetrics.ts.SIGTERMcloses ingress at once, running worker operations get 30 s, and the process force-exits at 60 s.deploy/azure/aca.yaml.docs/CONFIGURATION.mdgets the new row and names both wait settings, and the autoscaling list there now points to the new guide.README.mdandAGENTS.mdlink to it.Why the default stays 250 ms
We planned a 1000 ms default, then measured it. One replica with 2 GiB of memory and 2 CPUs, and clients that waited 1 s after each 503 as
Retry-After: 1asks:The ingress queue holds no more requests than ingress admits at once (
maxQueued: options.maxRequests, 8 per replica and 4 per tenant). A longer wait therefore cannot absorb a burst: everything beyond those 8 is refused at once. #269 tracks a deeper ingress queue. The guide's "Wait limits" section has the table.Notes for reviewers
deploy/azure/aca.yamlis unchanged. The guide explains that its scale rule of 50 concurrent requests per replica fires long after 503s begin, because one replica admits 8. Fixing the file is left for a separate change.Tests
SQL_FS_MAX_INGRESS_WAIT_MS=40are still queued at 39 ms. At 40 ms each gets a 503EWORKER_BUSYwithRetry-After: 1. These tests fail if either gate ignores the setting.SQL_FS_WORKER_QUEUE_WAIT_MSis 900. An explicit value wins.0,-1,1.5andNaN, stop startup.Verification
pnpm checkpassed: 2332 and 154 tests.pnpm verify:localpassed: contracts 94, output-cap 26, integration 265 (none skipped), SDK clients 102.Summary by cubic
Adds
SQL_FS_MAX_INGRESS_WAIT_MSto control how long a request or MCP listener waits for ingress capacity before a 503EWORKER_BUSY, fixes shutdown hanging on open MCP listener streams, and adds an autoscaling guide. The default stays the previous hard-coded 250 ms; measurements showed a longer wait cannot absorb bursts because the ingress queue caps at 8 requests per replica.GET /mcplistener streams first, so a client holding one open no longer blocks the drain until the 60 s force exit. In-flight MCP tool calls still finish, and terminating replicas refuse new connections. The newscripts/verify/shutdown.mjsasserts that SIGTERM with a listener open ends the stream cleanly and exits before the force-exit timer, andscripts/verify-local.shruns it.SQL_FS_MAX_INGRESS_WAIT_MSandSQL_FS_WORKER_QUEUE_WAIT_MSnow reject values above 2147483647 ms, which Node's timer would otherwise run after 1 ms. Invalid values such as0,-1,1.5andNaNalready stopped startup.docs/AUTOSCALING.mdis new and covers health and metrics endpoints, every exported metric, ranked scaling signals with example PromQL, measured single-replica capacity, shutdown draining, and an Azure Container Apps scale rule. It also lists the settings a replica needs when running 4 workers.docs/CONFIGURATION.md,README.md, andAGENTS.mdlink to it, anddocs/CONFIGURATION.mdadds the setting row.Written for commit 85c1fce. Summary will update on new commits.