fix(driver-sql): bound a connection attempt at 10s, correct the "no reconnection" claim (#3769) - #3781
Merged
Conversation
…econnection" claim (#3769, #3759) Two related corrections, both from measuring what #3741/#3751/#3765 asserted. ## The claim was wrong #3751 and #3765 shipped several statements that drivers never reconnect — "there is no lazy reconnection", "NOT retried and NOT reconnected", "stays disconnected for the process lifetime". Measured, both drivers recover: - driver-mongodb: killing a real mongod and restarting it on the same port, the SAME driver instance served the next write successfully (13ms) with no reconnect call from us — the official driver's topology monitor handles it. - driver-sql: a knex/pg pool is not poisoned by an outage. Its error tracks live server state (ECONNREFUSED while down → a handshake error once a listener is back → ECONNREFUSED again), i.e. every acquire opens a fresh connection. storage-driver.ts also sets pool.min = 0, so no stale idle connections. The original reasoning grepped this repo for `reconnect`, found nothing, and concluded recovery does not happen — but the recovery lives in the client libraries, not in our code. "We didn't write it" is not "it doesn't happen". ## Fail-fast at boot is unchanged — the reason is different It is not that the connection can never return; it is that the BOOT SEQUENCE never re-runs. A driver that missed init() also missed syncRegisteredSchemas(), so its tables can simply not exist even after the database comes back. The DEGRADED BOOT banner now says that instead of claiming a dead connection. ## The real defect underneath SqlDriver passed its config to knex untouched, so an endpoint that accepts TCP but never completes the handshake (overloaded instance, half-open firewall, LB mid-failover) made every query wait out tarn's 30s default, then fail with "Timeout acquiring a connection. The pool is probably full" — pointing an operator at pool sizing instead of the network. With a small pool.max a few such queries saturate the pool and everything else queues behind them. SqlDriver now defaults pool.createTimeoutMillis to 10s, matching driver-mongodb's existing `connectTimeoutMS ?? 10_000`. A host that sets its own createTimeoutMillis is left alone. Measured (black-holing listener, real knex/pg): today (no bound) 30024ms Knex: Timeout acquiring a connection… acquireConnectionTimeout 3s 3003ms Knex: Timeout acquiring a connection… pool.createTimeoutMillis 3s 3004ms Knex: Timeout acquiring a connection… connectionTimeoutMillis 3s 3006ms timeout expired Message accuracy needs the dialect-specific knob (last row), which changes the shape of `connection` and would regress serve.ts's startup banner — left to #3769. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T9VBCNvg9Zixrf1A9Rnwdd
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
size/m
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 18 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 28, 2026 05:20
This was referenced Jul 28, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 28, 2026
…r message (#3769) (#3791) #3781 bounded a connection attempt at 10s via pool.createTimeoutMillis, which stopped the 30s hang but kept knex's own wording: "Timeout acquiring a connection. The pool is probably full". The pool is not full — the server never completed the handshake — so that message sends an operator to tune pool.max while the network is what is broken. SqlDriver now also sets the dialect's own connect timeout, which fails with a message that names what happened: pg / postgres / postgresql / cockroachdb → connectionTimeoutMillis → "timeout expired" mysql / mysql2 → connectTimeout → "connect ETIMEDOUT" Carrying the timeout requires `connection` to be an object, so a URL string moves into the client's URL slot (connectionString for pg, uri for mysql2). Measured against a black-holing listener: both forms still reach the URL's own host/port and still honour ?sslmode=require. SQLite is untouched — a file open has no handshake to time out. A function-valued connection is left alone. The two bounds are deliberately unequal. They race and knex wins a tie, so equal values let the pool timeout fire first and the accurate message is never seen — caught by the black-hole test, which now asserts the wording rather than merely that something threw. The dialect timeout is the effective bound at 10s; the pool timeout is a strictly looser backstop, 10s → 15s. driver.config keeps the shape the author passed — the rewrite applies only to what knex receives. serve.ts's startup banner and createDatabase() both depend on that; a test pins it. createDatabase()'s own admin connection now gets the same bound. Also documents the two defaults in drivers.mdx, which had claimed the constructor takes a Knex.Config "verbatim".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
部分关闭 #3769,并订正 #3751 / #3765 里我基于错误前提写下的声明。#3759 已按 not_planned 关闭(前提被实测推翻)。
一、我之前的声明是错的
#3751 和 #3765 里我写了多处"driver 不会重连" ——
there is no lazy reconnection、NOT retried and NOT reconnected、stays disconnected for the process lifetime。实测下来两个驱动都会自行恢复。driver-mongodb(真 mongod,杀掉后原端口+原 dbpath 重启,同一个 driver 实例,我们不调用任何 reconnect):
driver-sql(TCP 层判定"池是否被永久毒死"):
每次获取都在开新连接。配置侧印证:
storage-driver.ts是pool.min = 0,不驻留空闲连接。我当初为什么错了: grep 自己的源码没看到
reconnect实现,就推断"不会恢复"。但恢复行为在底层客户端库里 —— mongodb 官方 driver 的拓扑监控、knex/tarn 的按需获取。"我们没写这段代码" ≠ "这个行为不会发生"。这是方法论错误,记在 #3759 的关闭说明里。边界: mongo 那个是真 mongod,结论硬;sql 那个没有真 postgres,证明的是"持续建新连接",不是"一次真实 failover 完整恢复"。
二、启动期 fail-fast 不变 —— 但理由变了
不是"连接再也回不来",而是启动序列不会重跑。错过
init()的 driver 同时错过了syncRegisteredSchemas(),所以即使数据库随后恢复,那些对象也可能压根没有表。DEGRADED BOOTbanner 现在说的是这个,而不是一句关于死连接的假话。订正了 4 处:
DriverConnectError消息、banner、resolveAllowDriverConnectFailure()文档、drivers/self-hosting 两个文档页。三、底下的真缺陷:连接尝试没有我们自己设的上限
SqlDriver把配置原样透传给 knex,于是面对接受 TCP 但不完成握手的端点(过载实例、半开防火墙、failover 中途的 LB),每个查询等满 tarn 默认的 30 秒,然后报Timeout acquiring a connection. The pool is probably full—— 池并没有满,运维会被引去调池子大小而不是查网络。pool.max: 5下几个这样的查询就把池占满,后续请求开始排队,那时候"pool is full"才变成真的,但那是继发症状。实测四种配置(真 knex/pg + 黑洞监听器):
SqlDriver现在默认pool.createTimeoutMillis = 10s,与 driver-mongodb 既有的connectTimeoutMS ?? 10_000对齐,两个驱动在同一点放弃。宿主自己设了就不动它。没在本 PR 做的: 消息准确性需要 dialect 专属的
connectionTimeoutMillis(上表最后一行),而它会把connection从字符串改成对象,导致serve.ts启动横幅的 URL 显示退化成(unknown)。留在 #3769,并已在代码注释里写明,免得下一个人以为已经解决。验证
sql-driver-connect-bound.test.ts(5 例):默认值生效 / 不覆盖宿主的min&max/ 宿主显式设置被尊重 / 真的限住了对黑洞端点的查询 / sqlite 正常连接不受干扰。engine-driver-connect-failfast.test.ts的 banner 断言 —— 从NOT reconnected改为SKIPPED FOR GOOD(持久后果是被跳过的 schema sync,不是死连接)。pnpm build71/71 绿。受影响的包全绿:types 26、driver-sql 330、objectql 1126、runtime 661、cli 667。关于本地全量
pnpm test,如实说明: 这台机器上它现在不稳定,干净的 main 也一样(131/132,dogfood 挂)。抓到的唯一真实失败是 cloud-connection 的 2 个用例 30s 超时,而那个文件自己的注释就写着 "cold import can eat several seconds on a fresh CI runner";turbo 随即取消其余在跑任务,所以每次"失败集合"都不同。4 核并行下的竞争,与本改动无关 —— 但这一条我没法在本地完全排除,以 CI 为准。关联
/ready探针)Generated by Claude Code