mirror of
https://github.com/gastownhall/beads.git
synced 2026-09-14 20:17:24 +08:00
Steve Yegge
f56632adcf
* perf(storage): stop burning two wasted MySQL sessions per bd invocation (wy-s8ytnw)
Every bd store-open against a shared dolt sql-server cost two sessions that
did no work:
1. readAndDial's data-port probe (internal/storage/dbproxy/proxy/endpoint.go)
dialed the proxy data port and closed it to "prove" it accepts. The proxy's
handleConn dials the BACKEND for every accepted client connection before a
byte flows, so the zero-byte probe was a full backend MySQL session on every
invocation from a proxied clone — and it proved nothing the authenticated
control-port identity reply had not: ListenAndServe binds the data listener
before the control listener exists. Drop the probe; the identity reply is
the liveness proof. (Deferring the backend dial until the first client byte
was considered and rejected: MySQL is server-speaks-first, so a real client
waits for the greeting and would deadlock.)
2. The no-database init connection. On the proxied/server uow path
(openAndInitSchema) and the dolt.New server path (openServerConnection) bd
opened a second, database-less pool per store-open purely to probe
existence (SHOW DATABASES / the schema-init preparation) before opening
the real pool. Both sites now try the database-scoped connection FIRST —
keyed the way the Gateway case already is, not by deleting the probe: a
successful connect is the existence proof, so the steady-state open (every
open but the very first) costs one session. Any connect failure (1049
Unknown database, server down, bad credentials) falls through to the
historical no-database path, which still owns creation, the #5042
ownership signal that arms FreshBootstrapHeal, databaseNotFoundError, and
every existing error message. `created` is honestly false on the fast
path; the uow provider reuses the init pool (nothing in the migration path
mutates session state).
New test TestReadAndDialAdoptsWithoutDialingDataPort pins the probe removal;
it fails (1 accept) against the previous endpoint.go and passes now.
Why the live server still saw a bare CREATE DATABASE per open despite
openServerConnection's SHOW DATABASES guard (the question on wy-s8ytnw): that
guard is not on the proxied path at all — the CREATE came from the uow
schema-init preparation, already fixed upstream by #6022 (alreadyConverged);
the deployed binary (71e5ad700) predates it (wy-7sx6cj).
* fix(storage): cap the fast-path probe budget; correct the pool-reuse premise
Maintainer review fixes for PR #6122. All four are non-gating minors (plus
one nit) from the review synthesis; the quality scorecard's decision was
approve with no required changes.
- uow fast path no longer costs a second retry budget (finding 1). openDB's
30s transient-ping budget was spent twice against an unreachable server:
once on the fast-path probe, whose error was discarded, and again on the
fall-through open. Measured 0s + 19.8s where it was 19.8s twice; the
returned "uow: ping db: ..." text is unchanged. The probe is now a single
bounded ping, mirroring the sibling fast path in internal/storage/dolt,
and the historical open keeps the retry budget it owned at base. Nothing
is lost by not retrying here: the endpoint is already live when
openAndInitSchema runs (GetCreateDatabaseProxyServerEndpoint), so the
probe was never the thing waiting out a starting server.
- Replace the pool-reuse comment's false premise (finding 2). "Nothing in
the migration path mutates session state" is wrong: six shipped
migrations clear FOREIGN_KEY_CHECKS and restore it, and the guarded ones
leave user variables set. State the real invariant instead — one pinned
session per attempt, paired restores inside each script, pool closed on
every initSchema failure — and record the forward requirement in
migrations/README.md, where migration authors will meet it.
- Log why either fast path fell through (finding 3), matching lock.go's
alreadyConverged idiom, so a fast path that has quietly stopped firing is
visible under BD_DEBUG instead of silently costing the second session.
- Amend both stale empty-DSN premise comments (finding 5, nit):
selectProbeDatabase and alreadyConverged now describe both entry shapes.
Finding 4 (a server-gated assertion on open-path selection) is left as the
follow-up that the synthesis and the scorecard both marked optional.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: plato <plato@ghosttrack.com>
Co-authored-by: Eddie the Engineer <julianknutsen@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>