perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX - #14776

Queued
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env
Queued

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX#14776
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14176

Adds OS_DATABASE_POOL_MAX, the operator-facing ceiling for a postgres / mysql datasource's knex pool, read in buildSqlPool.

The card's root cause is falsified; the defect is real

The card blames SqlDriver.withConnectBound for setting no pool size, so knex's {min:2,max:10} default applies. Measured on origin/main, that is not what the primary datasource runs:

  • buildSqlPool (packages/services/service-datasource/src/default-datasource-driver-factory.ts) hands everypostgres / mysql datasource an explicit {min: 0, max: 5} unless it declares its own pool block.
  • The primary datasource — the one behind OS_DATABASE_URL — is composed by the CLI as config: { url, ...autoMigrate } with no pool (packages/cli/src/utils/storage-driver.ts, the postgres arm).

So the effective per-replica ceiling was 5, not 10, and SqlDriver never sees an "unspecified" pool from this path — an env read in the driver would have been dead code behind the factory's explicit object. The card's own connection counts corroborate 5 over 10: 3 replicas x 5 + admin/sampler is about the ~21 observed; three pools saturated at 10 would have shown ~30.

This matches the correction already recorded on the issue and the maintainer ruling of 2026-09-02 (option A).

What changed

buildSqlPool now resolves max as declared pool.max > OS_DATABASE_POOL_MAX > 5. It is the only site that decides the unspecified case, so the precedence is expressed once, and the driver needs no knowledge of the factory's choice.

  • Unset changes nothing.{min: 0, max: 5}, byte-identical to today — that is the upgrade path for every existing deployment, and it is pinned by a test whose job is to go red if it ever drifts. A blank value reads as unset, so a declared-but-unfilled compose variable also keeps today's behaviour.
  • A bad value refuses the boot, naming the variable, the value it rejected and the sizing rule — instead of the lenient Number(process.env.X ?? default) shape, where a typo becomes NaN and the operator trying to raise the ceiling silently keeps the one they meant to leave. A pool ceiling is only ever measured in production.
  • OS_DATABASE_POOL_MIN is not exposed (ruling: this path already runs min: 0; a later patch if ever needed).
  • Named OS_DATABASE_* per AGENTS.md Prime Directive 9 — DATABASE is the existing family (OS_DATABASE_URL, OS_DATABASE_DRIVER, OS_DATABASE_SQLITE_JOURNAL_MODE); OS_DB_* has zero hits in the repo.

What I measured, and what I did not

⚠️The cluster throughput numbers are the reporter's, not mine. The ~25 rps plateau, the ~9-21 of 200 Postgres connections and the 77.8% 503 rate come from a live 3-replica EE cluster on 2026-09-01. There is no cluster and no live database here, and a fabricated local rerun would be evidence of nothing. I did not re-measure them. What is pinned instead is the mechanism those numbers rest on: which pool size actually reaches knex.

Measured here:

  • @objectstack/service-datasource suite: 0 failed | 635 passed without the new pin file, 0 failed | 643 passed (31 files) with it, at f82e7ada1.
  • tsc --noEmit exit 0, with --listFiles confirming both the changed source and the new test are in the program (this package's tsconfig includes src, tests and all).
  • Derived gate family (scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack): 59 commands, 55 exit 0. The 4 non-zero are all NOT MEASURED by their own verdict text, from unbuilt packages outside this diff's closure — check-test-completeness ("PREREQUISITE NOT MET ... ⛔ It is not a red"), check:dual-build-cjs-loads ("⛔ This is NOT a pass: nothing was measured"), check:type-check-debt ("⛔ This is NOT a pass and NOT a finding"), and check:skill-examples, which refuses because packages/client-react/dist is unbuilt and never reaches the docs surface.
  • Repo-wide pnpm lint (eslint . --no-inline-config): exit 0, full population, no narrowing.

Ablation — the knob is load-bearing. With the change committed, buildSqlPool was mutated to drop the env read (marker injected as a globalThis property, not a comment). The mutation was confirmed on disk before running: deleted-text occurrences 1 to 0, injected marker 1, blob 5efad548 to 47abe72b. The pins then read 4 failed | 4 passed: the four knob pins went red, while the unset-default pin, the blank-value pin, the declared-pool-wins pin and the unsupported-arm pin stayed green — the predicted direction, since none of those four depends on the env wiring. Restore proved by blob equality with HEAD (5efad548), empty git diff HEAD, and zero leftover markers.

Resolution path: the tests import the factory by relative path, so they read TypeScript source, not dist — no rebuild leg applies, and the ablation moving the result with no rebuild in between demonstrates it.

Not touched, deliberately

Clause-2: yes

Derived from the diff, not recalled: git diff -U0 origin/main...HEAD | grep -E '^\+\s*export ' returns nothing (zero new exported symbols), and no declared spec key is added (packages/spec/** untouched). The change reads env inside an existing non-exported function.

Those mechanical indicators point no, and I am reporting them because they are real information for the reviewer — but the grading is still yes: the widening is the documented operator-facing environment variable itself, a permanent public configuration obligation, which is exactly what the maintainer already graded yes when ruling option A. A mechanical export-grep should not argue down a ruling that considered this precise change, and no would have been the convenient answer here rather than the right one.

Changeset: minor on @objectstack/service-datasource — a new operator capability, backward-compatible, no default moved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

`buildSqlPool` gives every postgres/mysql datasource that declares no `pool`
an explicit `{min:0,max:5}`. The primary datasource — the one behind
`OS_DATABASE_URL` — is composed as a url and nothing else, so that `max: 5`
was the per-replica ceiling on every self-hosted deployment with no operator
knob for it. A driver-level env read would have been dead code behind this
function's explicit object.
Precedence: a declared `pool.max` > `OS_DATABASE_POOL_MAX` > today's `5`.
With the env unset nothing changes, which is the upgrade path for every
existing deployment and is pinned as such. A non-integer value refuses the
boot naming the variable, the value and the sizing rule.
Only the postgres/mysql arms call `buildSqlPool`, so the unsupported arms
(`memory` / `sqlite` / `sqlite-wasm` / `turso`) structurally cannot see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-datasource, touching 7 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/environment-variables.mdx(via OS_DATABASE_POOL_MAX (literal, a string literal in POOL_MAX_ENV))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 — the merge of head f82e7ada1f054467656ed84dfed9af37fc9a2012 into base df657d9df1fac5a2222f183008532613a1df585f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 && git checkout d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df657d9df1fac5a2222f183008532613a1df585f f82e7ada1f054467656ed84dfed9af37fc9a2012 && git checkout -B drift-repro df657d9df1fac5a2222f183008532613a1df585f && git merge --no-ff f82e7ada1f054467656ed84dfed9af37fc9a2012
node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs df657d9df1fac5a2222f183008532613a1df585f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-project-manager@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX - #14776

Queued
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env
Queued

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX#14776
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14176

Adds OS_DATABASE_POOL_MAX, the operator-facing ceiling for a postgres / mysql datasource's knex pool, read in buildSqlPool.

The card's root cause is falsified; the defect is real

The card blames SqlDriver.withConnectBound for setting no pool size, so knex's {min:2,max:10} default applies. Measured on origin/main, that is not what the primary datasource runs:

  • buildSqlPool (packages/services/service-datasource/src/default-datasource-driver-factory.ts) hands everypostgres / mysql datasource an explicit {min: 0, max: 5} unless it declares its own pool block.
  • The primary datasource — the one behind OS_DATABASE_URL — is composed by the CLI as config: { url, ...autoMigrate } with no pool (packages/cli/src/utils/storage-driver.ts, the postgres arm).

So the effective per-replica ceiling was 5, not 10, and SqlDriver never sees an "unspecified" pool from this path — an env read in the driver would have been dead code behind the factory's explicit object. The card's own connection counts corroborate 5 over 10: 3 replicas x 5 + admin/sampler is about the ~21 observed; three pools saturated at 10 would have shown ~30.

This matches the correction already recorded on the issue and the maintainer ruling of 2026-09-02 (option A).

What changed

buildSqlPool now resolves max as declared pool.max > OS_DATABASE_POOL_MAX > 5. It is the only site that decides the unspecified case, so the precedence is expressed once, and the driver needs no knowledge of the factory's choice.

  • Unset changes nothing.{min: 0, max: 5}, byte-identical to today — that is the upgrade path for every existing deployment, and it is pinned by a test whose job is to go red if it ever drifts. A blank value reads as unset, so a declared-but-unfilled compose variable also keeps today's behaviour.
  • A bad value refuses the boot, naming the variable, the value it rejected and the sizing rule — instead of the lenient Number(process.env.X ?? default) shape, where a typo becomes NaN and the operator trying to raise the ceiling silently keeps the one they meant to leave. A pool ceiling is only ever measured in production.
  • OS_DATABASE_POOL_MIN is not exposed (ruling: this path already runs min: 0; a later patch if ever needed).
  • Named OS_DATABASE_* per AGENTS.md Prime Directive 9 — DATABASE is the existing family (OS_DATABASE_URL, OS_DATABASE_DRIVER, OS_DATABASE_SQLITE_JOURNAL_MODE); OS_DB_* has zero hits in the repo.

What I measured, and what I did not

⚠️The cluster throughput numbers are the reporter's, not mine. The ~25 rps plateau, the ~9-21 of 200 Postgres connections and the 77.8% 503 rate come from a live 3-replica EE cluster on 2026-09-01. There is no cluster and no live database here, and a fabricated local rerun would be evidence of nothing. I did not re-measure them. What is pinned instead is the mechanism those numbers rest on: which pool size actually reaches knex.

Measured here:

  • @objectstack/service-datasource suite: 0 failed | 635 passed without the new pin file, 0 failed | 643 passed (31 files) with it, at f82e7ada1.
  • tsc --noEmit exit 0, with --listFiles confirming both the changed source and the new test are in the program (this package's tsconfig includes src, tests and all).
  • Derived gate family (scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack): 59 commands, 55 exit 0. The 4 non-zero are all NOT MEASURED by their own verdict text, from unbuilt packages outside this diff's closure — check-test-completeness ("PREREQUISITE NOT MET ... ⛔ It is not a red"), check:dual-build-cjs-loads ("⛔ This is NOT a pass: nothing was measured"), check:type-check-debt ("⛔ This is NOT a pass and NOT a finding"), and check:skill-examples, which refuses because packages/client-react/dist is unbuilt and never reaches the docs surface.
  • Repo-wide pnpm lint (eslint . --no-inline-config): exit 0, full population, no narrowing.

Ablation — the knob is load-bearing. With the change committed, buildSqlPool was mutated to drop the env read (marker injected as a globalThis property, not a comment). The mutation was confirmed on disk before running: deleted-text occurrences 1 to 0, injected marker 1, blob 5efad548 to 47abe72b. The pins then read 4 failed | 4 passed: the four knob pins went red, while the unset-default pin, the blank-value pin, the declared-pool-wins pin and the unsupported-arm pin stayed green — the predicted direction, since none of those four depends on the env wiring. Restore proved by blob equality with HEAD (5efad548), empty git diff HEAD, and zero leftover markers.

Resolution path: the tests import the factory by relative path, so they read TypeScript source, not dist — no rebuild leg applies, and the ablation moving the result with no rebuild in between demonstrates it.

Not touched, deliberately

Clause-2: yes

Derived from the diff, not recalled: git diff -U0 origin/main...HEAD | grep -E '^\+\s*export ' returns nothing (zero new exported symbols), and no declared spec key is added (packages/spec/** untouched). The change reads env inside an existing non-exported function.

Those mechanical indicators point no, and I am reporting them because they are real information for the reviewer — but the grading is still yes: the widening is the documented operator-facing environment variable itself, a permanent public configuration obligation, which is exactly what the maintainer already graded yes when ruling option A. A mechanical export-grep should not argue down a ruling that considered this precise change, and no would have been the convenient answer here rather than the right one.

Changeset: minor on @objectstack/service-datasource — a new operator capability, backward-compatible, no default moved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

`buildSqlPool` gives every postgres/mysql datasource that declares no `pool`
an explicit `{min:0,max:5}`. The primary datasource — the one behind
`OS_DATABASE_URL` — is composed as a url and nothing else, so that `max: 5`
was the per-replica ceiling on every self-hosted deployment with no operator
knob for it. A driver-level env read would have been dead code behind this
function's explicit object.
Precedence: a declared `pool.max` > `OS_DATABASE_POOL_MAX` > today's `5`.
With the env unset nothing changes, which is the upgrade path for every
existing deployment and is pinned as such. A non-integer value refuses the
boot naming the variable, the value and the sizing rule.
Only the postgres/mysql arms call `buildSqlPool`, so the unsupported arms
(`memory` / `sqlite` / `sqlite-wasm` / `turso`) structurally cannot see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-datasource, touching 7 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/environment-variables.mdx(via OS_DATABASE_POOL_MAX (literal, a string literal in POOL_MAX_ENV))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 — the merge of head f82e7ada1f054467656ed84dfed9af37fc9a2012 into base df657d9df1fac5a2222f183008532613a1df585f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 && git checkout d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df657d9df1fac5a2222f183008532613a1df585f f82e7ada1f054467656ed84dfed9af37fc9a2012 && git checkout -B drift-repro df657d9df1fac5a2222f183008532613a1df585f && git merge --no-ff f82e7ada1f054467656ed84dfed9af37fc9a2012
node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs df657d9df1fac5a2222f183008532613a1df585f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-project-manager@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX - #14776

Queued
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env
Queued

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX#14776
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14176

Adds OS_DATABASE_POOL_MAX, the operator-facing ceiling for a postgres / mysql datasource's knex pool, read in buildSqlPool.

The card's root cause is falsified; the defect is real

The card blames SqlDriver.withConnectBound for setting no pool size, so knex's {min:2,max:10} default applies. Measured on origin/main, that is not what the primary datasource runs:

  • buildSqlPool (packages/services/service-datasource/src/default-datasource-driver-factory.ts) hands everypostgres / mysql datasource an explicit {min: 0, max: 5} unless it declares its own pool block.
  • The primary datasource — the one behind OS_DATABASE_URL — is composed by the CLI as config: { url, ...autoMigrate } with no pool (packages/cli/src/utils/storage-driver.ts, the postgres arm).

So the effective per-replica ceiling was 5, not 10, and SqlDriver never sees an "unspecified" pool from this path — an env read in the driver would have been dead code behind the factory's explicit object. The card's own connection counts corroborate 5 over 10: 3 replicas x 5 + admin/sampler is about the ~21 observed; three pools saturated at 10 would have shown ~30.

This matches the correction already recorded on the issue and the maintainer ruling of 2026-09-02 (option A).

What changed

buildSqlPool now resolves max as declared pool.max > OS_DATABASE_POOL_MAX > 5. It is the only site that decides the unspecified case, so the precedence is expressed once, and the driver needs no knowledge of the factory's choice.

  • Unset changes nothing.{min: 0, max: 5}, byte-identical to today — that is the upgrade path for every existing deployment, and it is pinned by a test whose job is to go red if it ever drifts. A blank value reads as unset, so a declared-but-unfilled compose variable also keeps today's behaviour.
  • A bad value refuses the boot, naming the variable, the value it rejected and the sizing rule — instead of the lenient Number(process.env.X ?? default) shape, where a typo becomes NaN and the operator trying to raise the ceiling silently keeps the one they meant to leave. A pool ceiling is only ever measured in production.
  • OS_DATABASE_POOL_MIN is not exposed (ruling: this path already runs min: 0; a later patch if ever needed).
  • Named OS_DATABASE_* per AGENTS.md Prime Directive 9 — DATABASE is the existing family (OS_DATABASE_URL, OS_DATABASE_DRIVER, OS_DATABASE_SQLITE_JOURNAL_MODE); OS_DB_* has zero hits in the repo.

What I measured, and what I did not

⚠️The cluster throughput numbers are the reporter's, not mine. The ~25 rps plateau, the ~9-21 of 200 Postgres connections and the 77.8% 503 rate come from a live 3-replica EE cluster on 2026-09-01. There is no cluster and no live database here, and a fabricated local rerun would be evidence of nothing. I did not re-measure them. What is pinned instead is the mechanism those numbers rest on: which pool size actually reaches knex.

Measured here:

  • @objectstack/service-datasource suite: 0 failed | 635 passed without the new pin file, 0 failed | 643 passed (31 files) with it, at f82e7ada1.
  • tsc --noEmit exit 0, with --listFiles confirming both the changed source and the new test are in the program (this package's tsconfig includes src, tests and all).
  • Derived gate family (scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack): 59 commands, 55 exit 0. The 4 non-zero are all NOT MEASURED by their own verdict text, from unbuilt packages outside this diff's closure — check-test-completeness ("PREREQUISITE NOT MET ... ⛔ It is not a red"), check:dual-build-cjs-loads ("⛔ This is NOT a pass: nothing was measured"), check:type-check-debt ("⛔ This is NOT a pass and NOT a finding"), and check:skill-examples, which refuses because packages/client-react/dist is unbuilt and never reaches the docs surface.
  • Repo-wide pnpm lint (eslint . --no-inline-config): exit 0, full population, no narrowing.

Ablation — the knob is load-bearing. With the change committed, buildSqlPool was mutated to drop the env read (marker injected as a globalThis property, not a comment). The mutation was confirmed on disk before running: deleted-text occurrences 1 to 0, injected marker 1, blob 5efad548 to 47abe72b. The pins then read 4 failed | 4 passed: the four knob pins went red, while the unset-default pin, the blank-value pin, the declared-pool-wins pin and the unsupported-arm pin stayed green — the predicted direction, since none of those four depends on the env wiring. Restore proved by blob equality with HEAD (5efad548), empty git diff HEAD, and zero leftover markers.

Resolution path: the tests import the factory by relative path, so they read TypeScript source, not dist — no rebuild leg applies, and the ablation moving the result with no rebuild in between demonstrates it.

Not touched, deliberately

Clause-2: yes

Derived from the diff, not recalled: git diff -U0 origin/main...HEAD | grep -E '^\+\s*export ' returns nothing (zero new exported symbols), and no declared spec key is added (packages/spec/** untouched). The change reads env inside an existing non-exported function.

Those mechanical indicators point no, and I am reporting them because they are real information for the reviewer — but the grading is still yes: the widening is the documented operator-facing environment variable itself, a permanent public configuration obligation, which is exactly what the maintainer already graded yes when ruling option A. A mechanical export-grep should not argue down a ruling that considered this precise change, and no would have been the convenient answer here rather than the right one.

Changeset: minor on @objectstack/service-datasource — a new operator capability, backward-compatible, no default moved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

`buildSqlPool` gives every postgres/mysql datasource that declares no `pool`
an explicit `{min:0,max:5}`. The primary datasource — the one behind
`OS_DATABASE_URL` — is composed as a url and nothing else, so that `max: 5`
was the per-replica ceiling on every self-hosted deployment with no operator
knob for it. A driver-level env read would have been dead code behind this
function's explicit object.
Precedence: a declared `pool.max` > `OS_DATABASE_POOL_MAX` > today's `5`.
With the env unset nothing changes, which is the upgrade path for every
existing deployment and is pinned as such. A non-integer value refuses the
boot naming the variable, the value and the sizing rule.
Only the postgres/mysql arms call `buildSqlPool`, so the unsupported arms
(`memory` / `sqlite` / `sqlite-wasm` / `turso`) structurally cannot see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-datasource, touching 7 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/environment-variables.mdx(via OS_DATABASE_POOL_MAX (literal, a string literal in POOL_MAX_ENV))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 — the merge of head f82e7ada1f054467656ed84dfed9af37fc9a2012 into base df657d9df1fac5a2222f183008532613a1df585f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 && git checkout d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df657d9df1fac5a2222f183008532613a1df585f f82e7ada1f054467656ed84dfed9af37fc9a2012 && git checkout -B drift-repro df657d9df1fac5a2222f183008532613a1df585f && git merge --no-ff f82e7ada1f054467656ed84dfed9af37fc9a2012
node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs df657d9df1fac5a2222f183008532613a1df585f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-project-manager@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX - #14776

Queued
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env
Queued

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX#14776
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14176

Adds OS_DATABASE_POOL_MAX, the operator-facing ceiling for a postgres / mysql datasource's knex pool, read in buildSqlPool.

The card's root cause is falsified; the defect is real

The card blames SqlDriver.withConnectBound for setting no pool size, so knex's {min:2,max:10} default applies. Measured on origin/main, that is not what the primary datasource runs:

  • buildSqlPool (packages/services/service-datasource/src/default-datasource-driver-factory.ts) hands everypostgres / mysql datasource an explicit {min: 0, max: 5} unless it declares its own pool block.
  • The primary datasource — the one behind OS_DATABASE_URL — is composed by the CLI as config: { url, ...autoMigrate } with no pool (packages/cli/src/utils/storage-driver.ts, the postgres arm).

So the effective per-replica ceiling was 5, not 10, and SqlDriver never sees an "unspecified" pool from this path — an env read in the driver would have been dead code behind the factory's explicit object. The card's own connection counts corroborate 5 over 10: 3 replicas x 5 + admin/sampler is about the ~21 observed; three pools saturated at 10 would have shown ~30.

This matches the correction already recorded on the issue and the maintainer ruling of 2026-09-02 (option A).

What changed

buildSqlPool now resolves max as declared pool.max > OS_DATABASE_POOL_MAX > 5. It is the only site that decides the unspecified case, so the precedence is expressed once, and the driver needs no knowledge of the factory's choice.

  • Unset changes nothing.{min: 0, max: 5}, byte-identical to today — that is the upgrade path for every existing deployment, and it is pinned by a test whose job is to go red if it ever drifts. A blank value reads as unset, so a declared-but-unfilled compose variable also keeps today's behaviour.
  • A bad value refuses the boot, naming the variable, the value it rejected and the sizing rule — instead of the lenient Number(process.env.X ?? default) shape, where a typo becomes NaN and the operator trying to raise the ceiling silently keeps the one they meant to leave. A pool ceiling is only ever measured in production.
  • OS_DATABASE_POOL_MIN is not exposed (ruling: this path already runs min: 0; a later patch if ever needed).
  • Named OS_DATABASE_* per AGENTS.md Prime Directive 9 — DATABASE is the existing family (OS_DATABASE_URL, OS_DATABASE_DRIVER, OS_DATABASE_SQLITE_JOURNAL_MODE); OS_DB_* has zero hits in the repo.

What I measured, and what I did not

⚠️The cluster throughput numbers are the reporter's, not mine. The ~25 rps plateau, the ~9-21 of 200 Postgres connections and the 77.8% 503 rate come from a live 3-replica EE cluster on 2026-09-01. There is no cluster and no live database here, and a fabricated local rerun would be evidence of nothing. I did not re-measure them. What is pinned instead is the mechanism those numbers rest on: which pool size actually reaches knex.

Measured here:

  • @objectstack/service-datasource suite: 0 failed | 635 passed without the new pin file, 0 failed | 643 passed (31 files) with it, at f82e7ada1.
  • tsc --noEmit exit 0, with --listFiles confirming both the changed source and the new test are in the program (this package's tsconfig includes src, tests and all).
  • Derived gate family (scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack): 59 commands, 55 exit 0. The 4 non-zero are all NOT MEASURED by their own verdict text, from unbuilt packages outside this diff's closure — check-test-completeness ("PREREQUISITE NOT MET ... ⛔ It is not a red"), check:dual-build-cjs-loads ("⛔ This is NOT a pass: nothing was measured"), check:type-check-debt ("⛔ This is NOT a pass and NOT a finding"), and check:skill-examples, which refuses because packages/client-react/dist is unbuilt and never reaches the docs surface.
  • Repo-wide pnpm lint (eslint . --no-inline-config): exit 0, full population, no narrowing.

Ablation — the knob is load-bearing. With the change committed, buildSqlPool was mutated to drop the env read (marker injected as a globalThis property, not a comment). The mutation was confirmed on disk before running: deleted-text occurrences 1 to 0, injected marker 1, blob 5efad548 to 47abe72b. The pins then read 4 failed | 4 passed: the four knob pins went red, while the unset-default pin, the blank-value pin, the declared-pool-wins pin and the unsupported-arm pin stayed green — the predicted direction, since none of those four depends on the env wiring. Restore proved by blob equality with HEAD (5efad548), empty git diff HEAD, and zero leftover markers.

Resolution path: the tests import the factory by relative path, so they read TypeScript source, not dist — no rebuild leg applies, and the ablation moving the result with no rebuild in between demonstrates it.

Not touched, deliberately

Clause-2: yes

Derived from the diff, not recalled: git diff -U0 origin/main...HEAD | grep -E '^\+\s*export ' returns nothing (zero new exported symbols), and no declared spec key is added (packages/spec/** untouched). The change reads env inside an existing non-exported function.

Those mechanical indicators point no, and I am reporting them because they are real information for the reviewer — but the grading is still yes: the widening is the documented operator-facing environment variable itself, a permanent public configuration obligation, which is exactly what the maintainer already graded yes when ruling option A. A mechanical export-grep should not argue down a ruling that considered this precise change, and no would have been the convenient answer here rather than the right one.

Changeset: minor on @objectstack/service-datasource — a new operator capability, backward-compatible, no default moved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

`buildSqlPool` gives every postgres/mysql datasource that declares no `pool`
an explicit `{min:0,max:5}`. The primary datasource — the one behind
`OS_DATABASE_URL` — is composed as a url and nothing else, so that `max: 5`
was the per-replica ceiling on every self-hosted deployment with no operator
knob for it. A driver-level env read would have been dead code behind this
function's explicit object.
Precedence: a declared `pool.max` > `OS_DATABASE_POOL_MAX` > today's `5`.
With the env unset nothing changes, which is the upgrade path for every
existing deployment and is pinned as such. A non-integer value refuses the
boot naming the variable, the value and the sizing rule.
Only the postgres/mysql arms call `buildSqlPool`, so the unsupported arms
(`memory` / `sqlite` / `sqlite-wasm` / `turso`) structurally cannot see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-datasource, touching 7 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/environment-variables.mdx(via OS_DATABASE_POOL_MAX (literal, a string literal in POOL_MAX_ENV))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 — the merge of head f82e7ada1f054467656ed84dfed9af37fc9a2012 into base df657d9df1fac5a2222f183008532613a1df585f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 && git checkout d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df657d9df1fac5a2222f183008532613a1df585f f82e7ada1f054467656ed84dfed9af37fc9a2012 && git checkout -B drift-repro df657d9df1fac5a2222f183008532613a1df585f && git merge --no-ff f82e7ada1f054467656ed84dfed9af37fc9a2012
node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs df657d9df1fac5a2222f183008532613a1df585f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-project-manager@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX - #14776

Queued
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env
Queued

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX#14776
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14176

Adds OS_DATABASE_POOL_MAX, the operator-facing ceiling for a postgres / mysql datasource's knex pool, read in buildSqlPool.

The card's root cause is falsified; the defect is real

The card blames SqlDriver.withConnectBound for setting no pool size, so knex's {min:2,max:10} default applies. Measured on origin/main, that is not what the primary datasource runs:

  • buildSqlPool (packages/services/service-datasource/src/default-datasource-driver-factory.ts) hands everypostgres / mysql datasource an explicit {min: 0, max: 5} unless it declares its own pool block.
  • The primary datasource — the one behind OS_DATABASE_URL — is composed by the CLI as config: { url, ...autoMigrate } with no pool (packages/cli/src/utils/storage-driver.ts, the postgres arm).

So the effective per-replica ceiling was 5, not 10, and SqlDriver never sees an "unspecified" pool from this path — an env read in the driver would have been dead code behind the factory's explicit object. The card's own connection counts corroborate 5 over 10: 3 replicas x 5 + admin/sampler is about the ~21 observed; three pools saturated at 10 would have shown ~30.

This matches the correction already recorded on the issue and the maintainer ruling of 2026-09-02 (option A).

What changed

buildSqlPool now resolves max as declared pool.max > OS_DATABASE_POOL_MAX > 5. It is the only site that decides the unspecified case, so the precedence is expressed once, and the driver needs no knowledge of the factory's choice.

  • Unset changes nothing.{min: 0, max: 5}, byte-identical to today — that is the upgrade path for every existing deployment, and it is pinned by a test whose job is to go red if it ever drifts. A blank value reads as unset, so a declared-but-unfilled compose variable also keeps today's behaviour.
  • A bad value refuses the boot, naming the variable, the value it rejected and the sizing rule — instead of the lenient Number(process.env.X ?? default) shape, where a typo becomes NaN and the operator trying to raise the ceiling silently keeps the one they meant to leave. A pool ceiling is only ever measured in production.
  • OS_DATABASE_POOL_MIN is not exposed (ruling: this path already runs min: 0; a later patch if ever needed).
  • Named OS_DATABASE_* per AGENTS.md Prime Directive 9 — DATABASE is the existing family (OS_DATABASE_URL, OS_DATABASE_DRIVER, OS_DATABASE_SQLITE_JOURNAL_MODE); OS_DB_* has zero hits in the repo.

What I measured, and what I did not

⚠️The cluster throughput numbers are the reporter's, not mine. The ~25 rps plateau, the ~9-21 of 200 Postgres connections and the 77.8% 503 rate come from a live 3-replica EE cluster on 2026-09-01. There is no cluster and no live database here, and a fabricated local rerun would be evidence of nothing. I did not re-measure them. What is pinned instead is the mechanism those numbers rest on: which pool size actually reaches knex.

Measured here:

  • @objectstack/service-datasource suite: 0 failed | 635 passed without the new pin file, 0 failed | 643 passed (31 files) with it, at f82e7ada1.
  • tsc --noEmit exit 0, with --listFiles confirming both the changed source and the new test are in the program (this package's tsconfig includes src, tests and all).
  • Derived gate family (scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack): 59 commands, 55 exit 0. The 4 non-zero are all NOT MEASURED by their own verdict text, from unbuilt packages outside this diff's closure — check-test-completeness ("PREREQUISITE NOT MET ... ⛔ It is not a red"), check:dual-build-cjs-loads ("⛔ This is NOT a pass: nothing was measured"), check:type-check-debt ("⛔ This is NOT a pass and NOT a finding"), and check:skill-examples, which refuses because packages/client-react/dist is unbuilt and never reaches the docs surface.
  • Repo-wide pnpm lint (eslint . --no-inline-config): exit 0, full population, no narrowing.

Ablation — the knob is load-bearing. With the change committed, buildSqlPool was mutated to drop the env read (marker injected as a globalThis property, not a comment). The mutation was confirmed on disk before running: deleted-text occurrences 1 to 0, injected marker 1, blob 5efad548 to 47abe72b. The pins then read 4 failed | 4 passed: the four knob pins went red, while the unset-default pin, the blank-value pin, the declared-pool-wins pin and the unsupported-arm pin stayed green — the predicted direction, since none of those four depends on the env wiring. Restore proved by blob equality with HEAD (5efad548), empty git diff HEAD, and zero leftover markers.

Resolution path: the tests import the factory by relative path, so they read TypeScript source, not dist — no rebuild leg applies, and the ablation moving the result with no rebuild in between demonstrates it.

Not touched, deliberately

Clause-2: yes

Derived from the diff, not recalled: git diff -U0 origin/main...HEAD | grep -E '^\+\s*export ' returns nothing (zero new exported symbols), and no declared spec key is added (packages/spec/** untouched). The change reads env inside an existing non-exported function.

Those mechanical indicators point no, and I am reporting them because they are real information for the reviewer — but the grading is still yes: the widening is the documented operator-facing environment variable itself, a permanent public configuration obligation, which is exactly what the maintainer already graded yes when ruling option A. A mechanical export-grep should not argue down a ruling that considered this precise change, and no would have been the convenient answer here rather than the right one.

Changeset: minor on @objectstack/service-datasource — a new operator capability, backward-compatible, no default moved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

`buildSqlPool` gives every postgres/mysql datasource that declares no `pool`
an explicit `{min:0,max:5}`. The primary datasource — the one behind
`OS_DATABASE_URL` — is composed as a url and nothing else, so that `max: 5`
was the per-replica ceiling on every self-hosted deployment with no operator
knob for it. A driver-level env read would have been dead code behind this
function's explicit object.
Precedence: a declared `pool.max` > `OS_DATABASE_POOL_MAX` > today's `5`.
With the env unset nothing changes, which is the upgrade path for every
existing deployment and is pinned as such. A non-integer value refuses the
boot naming the variable, the value and the sizing rule.
Only the postgres/mysql arms call `buildSqlPool`, so the unsupported arms
(`memory` / `sqlite` / `sqlite-wasm` / `turso`) structurally cannot see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-datasource, touching 7 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/environment-variables.mdx(via OS_DATABASE_POOL_MAX (literal, a string literal in POOL_MAX_ENV))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 — the merge of head f82e7ada1f054467656ed84dfed9af37fc9a2012 into base df657d9df1fac5a2222f183008532613a1df585f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 && git checkout d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df657d9df1fac5a2222f183008532613a1df585f f82e7ada1f054467656ed84dfed9af37fc9a2012 && git checkout -B drift-repro df657d9df1fac5a2222f183008532613a1df585f && git merge --no-ff f82e7ada1f054467656ed84dfed9af37fc9a2012
node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs df657d9df1fac5a2222f183008532613a1df585f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-project-manager@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX - #14776

Queued
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env
Queued

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX#14776
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14176

Adds OS_DATABASE_POOL_MAX, the operator-facing ceiling for a postgres / mysql datasource's knex pool, read in buildSqlPool.

The card's root cause is falsified; the defect is real

The card blames SqlDriver.withConnectBound for setting no pool size, so knex's {min:2,max:10} default applies. Measured on origin/main, that is not what the primary datasource runs:

  • buildSqlPool (packages/services/service-datasource/src/default-datasource-driver-factory.ts) hands everypostgres / mysql datasource an explicit {min: 0, max: 5} unless it declares its own pool block.
  • The primary datasource — the one behind OS_DATABASE_URL — is composed by the CLI as config: { url, ...autoMigrate } with no pool (packages/cli/src/utils/storage-driver.ts, the postgres arm).

So the effective per-replica ceiling was 5, not 10, and SqlDriver never sees an "unspecified" pool from this path — an env read in the driver would have been dead code behind the factory's explicit object. The card's own connection counts corroborate 5 over 10: 3 replicas x 5 + admin/sampler is about the ~21 observed; three pools saturated at 10 would have shown ~30.

This matches the correction already recorded on the issue and the maintainer ruling of 2026-09-02 (option A).

What changed

buildSqlPool now resolves max as declared pool.max > OS_DATABASE_POOL_MAX > 5. It is the only site that decides the unspecified case, so the precedence is expressed once, and the driver needs no knowledge of the factory's choice.

  • Unset changes nothing.{min: 0, max: 5}, byte-identical to today — that is the upgrade path for every existing deployment, and it is pinned by a test whose job is to go red if it ever drifts. A blank value reads as unset, so a declared-but-unfilled compose variable also keeps today's behaviour.
  • A bad value refuses the boot, naming the variable, the value it rejected and the sizing rule — instead of the lenient Number(process.env.X ?? default) shape, where a typo becomes NaN and the operator trying to raise the ceiling silently keeps the one they meant to leave. A pool ceiling is only ever measured in production.
  • OS_DATABASE_POOL_MIN is not exposed (ruling: this path already runs min: 0; a later patch if ever needed).
  • Named OS_DATABASE_* per AGENTS.md Prime Directive 9 — DATABASE is the existing family (OS_DATABASE_URL, OS_DATABASE_DRIVER, OS_DATABASE_SQLITE_JOURNAL_MODE); OS_DB_* has zero hits in the repo.

What I measured, and what I did not

⚠️The cluster throughput numbers are the reporter's, not mine. The ~25 rps plateau, the ~9-21 of 200 Postgres connections and the 77.8% 503 rate come from a live 3-replica EE cluster on 2026-09-01. There is no cluster and no live database here, and a fabricated local rerun would be evidence of nothing. I did not re-measure them. What is pinned instead is the mechanism those numbers rest on: which pool size actually reaches knex.

Measured here:

  • @objectstack/service-datasource suite: 0 failed | 635 passed without the new pin file, 0 failed | 643 passed (31 files) with it, at f82e7ada1.
  • tsc --noEmit exit 0, with --listFiles confirming both the changed source and the new test are in the program (this package's tsconfig includes src, tests and all).
  • Derived gate family (scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack): 59 commands, 55 exit 0. The 4 non-zero are all NOT MEASURED by their own verdict text, from unbuilt packages outside this diff's closure — check-test-completeness ("PREREQUISITE NOT MET ... ⛔ It is not a red"), check:dual-build-cjs-loads ("⛔ This is NOT a pass: nothing was measured"), check:type-check-debt ("⛔ This is NOT a pass and NOT a finding"), and check:skill-examples, which refuses because packages/client-react/dist is unbuilt and never reaches the docs surface.
  • Repo-wide pnpm lint (eslint . --no-inline-config): exit 0, full population, no narrowing.

Ablation — the knob is load-bearing. With the change committed, buildSqlPool was mutated to drop the env read (marker injected as a globalThis property, not a comment). The mutation was confirmed on disk before running: deleted-text occurrences 1 to 0, injected marker 1, blob 5efad548 to 47abe72b. The pins then read 4 failed | 4 passed: the four knob pins went red, while the unset-default pin, the blank-value pin, the declared-pool-wins pin and the unsupported-arm pin stayed green — the predicted direction, since none of those four depends on the env wiring. Restore proved by blob equality with HEAD (5efad548), empty git diff HEAD, and zero leftover markers.

Resolution path: the tests import the factory by relative path, so they read TypeScript source, not dist — no rebuild leg applies, and the ablation moving the result with no rebuild in between demonstrates it.

Not touched, deliberately

Clause-2: yes

Derived from the diff, not recalled: git diff -U0 origin/main...HEAD | grep -E '^\+\s*export ' returns nothing (zero new exported symbols), and no declared spec key is added (packages/spec/** untouched). The change reads env inside an existing non-exported function.

Those mechanical indicators point no, and I am reporting them because they are real information for the reviewer — but the grading is still yes: the widening is the documented operator-facing environment variable itself, a permanent public configuration obligation, which is exactly what the maintainer already graded yes when ruling option A. A mechanical export-grep should not argue down a ruling that considered this precise change, and no would have been the convenient answer here rather than the right one.

Changeset: minor on @objectstack/service-datasource — a new operator capability, backward-compatible, no default moved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

`buildSqlPool` gives every postgres/mysql datasource that declares no `pool`
an explicit `{min:0,max:5}`. The primary datasource — the one behind
`OS_DATABASE_URL` — is composed as a url and nothing else, so that `max: 5`
was the per-replica ceiling on every self-hosted deployment with no operator
knob for it. A driver-level env read would have been dead code behind this
function's explicit object.
Precedence: a declared `pool.max` > `OS_DATABASE_POOL_MAX` > today's `5`.
With the env unset nothing changes, which is the upgrade path for every
existing deployment and is pinned as such. A non-integer value refuses the
boot naming the variable, the value and the sizing rule.
Only the postgres/mysql arms call `buildSqlPool`, so the unsupported arms
(`memory` / `sqlite` / `sqlite-wasm` / `turso`) structurally cannot see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-datasource, touching 7 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/environment-variables.mdx(via OS_DATABASE_POOL_MAX (literal, a string literal in POOL_MAX_ENV))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 — the merge of head f82e7ada1f054467656ed84dfed9af37fc9a2012 into base df657d9df1fac5a2222f183008532613a1df585f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 && git checkout d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df657d9df1fac5a2222f183008532613a1df585f f82e7ada1f054467656ed84dfed9af37fc9a2012 && git checkout -B drift-repro df657d9df1fac5a2222f183008532613a1df585f && git merge --no-ff f82e7ada1f054467656ed84dfed9af37fc9a2012
node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs df657d9df1fac5a2222f183008532613a1df585f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-project-manager@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX - #14776

Queued
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env
Queued

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX#14776
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14176

Adds OS_DATABASE_POOL_MAX, the operator-facing ceiling for a postgres / mysql datasource's knex pool, read in buildSqlPool.

The card's root cause is falsified; the defect is real

The card blames SqlDriver.withConnectBound for setting no pool size, so knex's {min:2,max:10} default applies. Measured on origin/main, that is not what the primary datasource runs:

  • buildSqlPool (packages/services/service-datasource/src/default-datasource-driver-factory.ts) hands everypostgres / mysql datasource an explicit {min: 0, max: 5} unless it declares its own pool block.
  • The primary datasource — the one behind OS_DATABASE_URL — is composed by the CLI as config: { url, ...autoMigrate } with no pool (packages/cli/src/utils/storage-driver.ts, the postgres arm).

So the effective per-replica ceiling was 5, not 10, and SqlDriver never sees an "unspecified" pool from this path — an env read in the driver would have been dead code behind the factory's explicit object. The card's own connection counts corroborate 5 over 10: 3 replicas x 5 + admin/sampler is about the ~21 observed; three pools saturated at 10 would have shown ~30.

This matches the correction already recorded on the issue and the maintainer ruling of 2026-09-02 (option A).

What changed

buildSqlPool now resolves max as declared pool.max > OS_DATABASE_POOL_MAX > 5. It is the only site that decides the unspecified case, so the precedence is expressed once, and the driver needs no knowledge of the factory's choice.

  • Unset changes nothing.{min: 0, max: 5}, byte-identical to today — that is the upgrade path for every existing deployment, and it is pinned by a test whose job is to go red if it ever drifts. A blank value reads as unset, so a declared-but-unfilled compose variable also keeps today's behaviour.
  • A bad value refuses the boot, naming the variable, the value it rejected and the sizing rule — instead of the lenient Number(process.env.X ?? default) shape, where a typo becomes NaN and the operator trying to raise the ceiling silently keeps the one they meant to leave. A pool ceiling is only ever measured in production.
  • OS_DATABASE_POOL_MIN is not exposed (ruling: this path already runs min: 0; a later patch if ever needed).
  • Named OS_DATABASE_* per AGENTS.md Prime Directive 9 — DATABASE is the existing family (OS_DATABASE_URL, OS_DATABASE_DRIVER, OS_DATABASE_SQLITE_JOURNAL_MODE); OS_DB_* has zero hits in the repo.

What I measured, and what I did not

⚠️The cluster throughput numbers are the reporter's, not mine. The ~25 rps plateau, the ~9-21 of 200 Postgres connections and the 77.8% 503 rate come from a live 3-replica EE cluster on 2026-09-01. There is no cluster and no live database here, and a fabricated local rerun would be evidence of nothing. I did not re-measure them. What is pinned instead is the mechanism those numbers rest on: which pool size actually reaches knex.

Measured here:

  • @objectstack/service-datasource suite: 0 failed | 635 passed without the new pin file, 0 failed | 643 passed (31 files) with it, at f82e7ada1.
  • tsc --noEmit exit 0, with --listFiles confirming both the changed source and the new test are in the program (this package's tsconfig includes src, tests and all).
  • Derived gate family (scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack): 59 commands, 55 exit 0. The 4 non-zero are all NOT MEASURED by their own verdict text, from unbuilt packages outside this diff's closure — check-test-completeness ("PREREQUISITE NOT MET ... ⛔ It is not a red"), check:dual-build-cjs-loads ("⛔ This is NOT a pass: nothing was measured"), check:type-check-debt ("⛔ This is NOT a pass and NOT a finding"), and check:skill-examples, which refuses because packages/client-react/dist is unbuilt and never reaches the docs surface.
  • Repo-wide pnpm lint (eslint . --no-inline-config): exit 0, full population, no narrowing.

Ablation — the knob is load-bearing. With the change committed, buildSqlPool was mutated to drop the env read (marker injected as a globalThis property, not a comment). The mutation was confirmed on disk before running: deleted-text occurrences 1 to 0, injected marker 1, blob 5efad548 to 47abe72b. The pins then read 4 failed | 4 passed: the four knob pins went red, while the unset-default pin, the blank-value pin, the declared-pool-wins pin and the unsupported-arm pin stayed green — the predicted direction, since none of those four depends on the env wiring. Restore proved by blob equality with HEAD (5efad548), empty git diff HEAD, and zero leftover markers.

Resolution path: the tests import the factory by relative path, so they read TypeScript source, not dist — no rebuild leg applies, and the ablation moving the result with no rebuild in between demonstrates it.

Not touched, deliberately

Clause-2: yes

Derived from the diff, not recalled: git diff -U0 origin/main...HEAD | grep -E '^\+\s*export ' returns nothing (zero new exported symbols), and no declared spec key is added (packages/spec/** untouched). The change reads env inside an existing non-exported function.

Those mechanical indicators point no, and I am reporting them because they are real information for the reviewer — but the grading is still yes: the widening is the documented operator-facing environment variable itself, a permanent public configuration obligation, which is exactly what the maintainer already graded yes when ruling option A. A mechanical export-grep should not argue down a ruling that considered this precise change, and no would have been the convenient answer here rather than the right one.

Changeset: minor on @objectstack/service-datasource — a new operator capability, backward-compatible, no default moved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

`buildSqlPool` gives every postgres/mysql datasource that declares no `pool`
an explicit `{min:0,max:5}`. The primary datasource — the one behind
`OS_DATABASE_URL` — is composed as a url and nothing else, so that `max: 5`
was the per-replica ceiling on every self-hosted deployment with no operator
knob for it. A driver-level env read would have been dead code behind this
function's explicit object.
Precedence: a declared `pool.max` > `OS_DATABASE_POOL_MAX` > today's `5`.
With the env unset nothing changes, which is the upgrade path for every
existing deployment and is pinned as such. A non-integer value refuses the
boot naming the variable, the value and the sizing rule.
Only the postgres/mysql arms call `buildSqlPool`, so the unsupported arms
(`memory` / `sqlite` / `sqlite-wasm` / `turso`) structurally cannot see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-datasource, touching 7 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/environment-variables.mdx(via OS_DATABASE_POOL_MAX (literal, a string literal in POOL_MAX_ENV))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 — the merge of head f82e7ada1f054467656ed84dfed9af37fc9a2012 into base df657d9df1fac5a2222f183008532613a1df585f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 && git checkout d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df657d9df1fac5a2222f183008532613a1df585f f82e7ada1f054467656ed84dfed9af37fc9a2012 && git checkout -B drift-repro df657d9df1fac5a2222f183008532613a1df585f && git merge --no-ff f82e7ada1f054467656ed84dfed9af37fc9a2012
node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs df657d9df1fac5a2222f183008532613a1df585f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-project-manager@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX - #14776

Queued
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env
Queued

perf(datasource): size the primary SQL pool from OS_DATABASE_POOL_MAX#14776
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-14176-primary-datasource-pool-env

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14176

Adds OS_DATABASE_POOL_MAX, the operator-facing ceiling for a postgres / mysql datasource's knex pool, read in buildSqlPool.

The card's root cause is falsified; the defect is real

The card blames SqlDriver.withConnectBound for setting no pool size, so knex's {min:2,max:10} default applies. Measured on origin/main, that is not what the primary datasource runs:

  • buildSqlPool (packages/services/service-datasource/src/default-datasource-driver-factory.ts) hands everypostgres / mysql datasource an explicit {min: 0, max: 5} unless it declares its own pool block.
  • The primary datasource — the one behind OS_DATABASE_URL — is composed by the CLI as config: { url, ...autoMigrate } with no pool (packages/cli/src/utils/storage-driver.ts, the postgres arm).

So the effective per-replica ceiling was 5, not 10, and SqlDriver never sees an "unspecified" pool from this path — an env read in the driver would have been dead code behind the factory's explicit object. The card's own connection counts corroborate 5 over 10: 3 replicas x 5 + admin/sampler is about the ~21 observed; three pools saturated at 10 would have shown ~30.

This matches the correction already recorded on the issue and the maintainer ruling of 2026-09-02 (option A).

What changed

buildSqlPool now resolves max as declared pool.max > OS_DATABASE_POOL_MAX > 5. It is the only site that decides the unspecified case, so the precedence is expressed once, and the driver needs no knowledge of the factory's choice.

  • Unset changes nothing.{min: 0, max: 5}, byte-identical to today — that is the upgrade path for every existing deployment, and it is pinned by a test whose job is to go red if it ever drifts. A blank value reads as unset, so a declared-but-unfilled compose variable also keeps today's behaviour.
  • A bad value refuses the boot, naming the variable, the value it rejected and the sizing rule — instead of the lenient Number(process.env.X ?? default) shape, where a typo becomes NaN and the operator trying to raise the ceiling silently keeps the one they meant to leave. A pool ceiling is only ever measured in production.
  • OS_DATABASE_POOL_MIN is not exposed (ruling: this path already runs min: 0; a later patch if ever needed).
  • Named OS_DATABASE_* per AGENTS.md Prime Directive 9 — DATABASE is the existing family (OS_DATABASE_URL, OS_DATABASE_DRIVER, OS_DATABASE_SQLITE_JOURNAL_MODE); OS_DB_* has zero hits in the repo.

What I measured, and what I did not

⚠️The cluster throughput numbers are the reporter's, not mine. The ~25 rps plateau, the ~9-21 of 200 Postgres connections and the 77.8% 503 rate come from a live 3-replica EE cluster on 2026-09-01. There is no cluster and no live database here, and a fabricated local rerun would be evidence of nothing. I did not re-measure them. What is pinned instead is the mechanism those numbers rest on: which pool size actually reaches knex.

Measured here:

  • @objectstack/service-datasource suite: 0 failed | 635 passed without the new pin file, 0 failed | 643 passed (31 files) with it, at f82e7ada1.
  • tsc --noEmit exit 0, with --listFiles confirming both the changed source and the new test are in the program (this package's tsconfig includes src, tests and all).
  • Derived gate family (scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack): 59 commands, 55 exit 0. The 4 non-zero are all NOT MEASURED by their own verdict text, from unbuilt packages outside this diff's closure — check-test-completeness ("PREREQUISITE NOT MET ... ⛔ It is not a red"), check:dual-build-cjs-loads ("⛔ This is NOT a pass: nothing was measured"), check:type-check-debt ("⛔ This is NOT a pass and NOT a finding"), and check:skill-examples, which refuses because packages/client-react/dist is unbuilt and never reaches the docs surface.
  • Repo-wide pnpm lint (eslint . --no-inline-config): exit 0, full population, no narrowing.

Ablation — the knob is load-bearing. With the change committed, buildSqlPool was mutated to drop the env read (marker injected as a globalThis property, not a comment). The mutation was confirmed on disk before running: deleted-text occurrences 1 to 0, injected marker 1, blob 5efad548 to 47abe72b. The pins then read 4 failed | 4 passed: the four knob pins went red, while the unset-default pin, the blank-value pin, the declared-pool-wins pin and the unsupported-arm pin stayed green — the predicted direction, since none of those four depends on the env wiring. Restore proved by blob equality with HEAD (5efad548), empty git diff HEAD, and zero leftover markers.

Resolution path: the tests import the factory by relative path, so they read TypeScript source, not dist — no rebuild leg applies, and the ablation moving the result with no rebuild in between demonstrates it.

Not touched, deliberately

Clause-2: yes

Derived from the diff, not recalled: git diff -U0 origin/main...HEAD | grep -E '^\+\s*export ' returns nothing (zero new exported symbols), and no declared spec key is added (packages/spec/** untouched). The change reads env inside an existing non-exported function.

Those mechanical indicators point no, and I am reporting them because they are real information for the reviewer — but the grading is still yes: the widening is the documented operator-facing environment variable itself, a permanent public configuration obligation, which is exactly what the maintainer already graded yes when ruling option A. A mechanical export-grep should not argue down a ruling that considered this precise change, and no would have been the convenient answer here rather than the right one.

Changeset: minor on @objectstack/service-datasource — a new operator capability, backward-compatible, no default moved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8


Generated by Claude Code

`buildSqlPool` gives every postgres/mysql datasource that declares no `pool`
an explicit `{min:0,max:5}`. The primary datasource — the one behind
`OS_DATABASE_URL` — is composed as a url and nothing else, so that `max: 5`
was the per-replica ceiling on every self-hosted deployment with no operator
knob for it. A driver-level env read would have been dead code behind this
function's explicit object.
Precedence: a declared `pool.max` > `OS_DATABASE_POOL_MAX` > today's `5`.
With the env unset nothing changes, which is the upgrade path for every
existing deployment and is pinned as such. A non-integer value refuses the
boot naming the variable, the value and the sizing rule.
Only the postgres/mysql arms call `buildSqlPool`, so the unsupported arms
(`memory` / `sqlite` / `sqlite-wasm` / `turso`) structurally cannot see it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-datasource, touching 7 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/environment-variables.mdx(via OS_DATABASE_POOL_MAX (literal, a string literal in POOL_MAX_ENV))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 — the merge of head f82e7ada1f054467656ed84dfed9af37fc9a2012 into base df657d9df1fac5a2222f183008532613a1df585f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3 && git checkout d3d5c70ab838c5a0cf444f6b6a7cd77e247124e3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df657d9df1fac5a2222f183008532613a1df585f f82e7ada1f054467656ed84dfed9af37fc9a2012 && git checkout -B drift-repro df657d9df1fac5a2222f183008532613a1df585f && git merge --no-ff f82e7ada1f054467656ed84dfed9af37fc9a2012
node scripts/docs-audit/affected-docs.mjs --json df657d9df1fac5a2222f183008532613a1df585f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs df657d9df1fac5a2222f183008532613a1df585f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-project-manager@claude