Fixed racy first-start database initialization#2919
Conversation
|
|
SummaryThis run exercised first-run startup and bootstrap behavior end to end, including connection gating, default database creation, readiness timing, retry handling, and restart recovery under failure conditions. Overall core startup and driver initialization flows look healthy, with issues concentrated in edge-case failure windows rather than normal initialization. Merge with caution — one medium-severity failure is attributable to this PR and indicates a startup-ordering gap where the service can appear ready and accept real queries before full initialization viability is confirmed. Other failures are non-attributable caveats in existing bootstrap error-handling paths and are not the primary merge blocker for this change. Tests run by ItoAdditional Findings DetailsThese findings are unrelated to the current changes but were observed during testing. 🟡 Retry fails after interrupted database bootstrap
Evidence Package⚪ Malformed DOLTGRES_DB input leaves partial database side effects
Evidence PackageTip Reply with @itoqa to send us feedback on this test run. |
| @@ -180,32 +192,54 @@ func runServer(ctx context.Context, cfg *servercfg.DoltgresConfig, dEnv *env.Dol | |||
| } | |||
|
|
|||
| if initializeDefaultDatabase { | |||
There was a problem hiding this comment.
Startup gate opens before replication viability is known
What failed: The gate is released before replication startup is validated, so the process accepts external traffic during a transient window where overall startup is not yet viable.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: Medium
- Impact: When replication startup fails, the server can briefly accept real client queries before shutting down, causing interrupted sessions and unreliable startup behavior for database users.
- Steps to Reproduce:
- Start the server with an empty data directory so first-run default database initialization runs under the startup gate.
- Configure replication to a non-routable upstream (for example
10.255.255.1) sostartReplication()cannot establish viability promptly. - Race external
psqlconnections immediately after startup and observe successful queries before replication startup later fails/stalls.
- Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
- Code Analysis: In
server/server.go, the PR-modified startup path callscreateDefaultDatabase(ssCfg, gate), thengate.Release(), and only afterward runsstartReplication(cfg, ssCfg). If replication initialization blocks or fails, external connections have already been allowed. Runtime evidence matches this ordering: logs showStarting replicationfollowed by externalNewConnectionIDs 2-8, and capturedpsqlqueries (SELECT 1,SELECT version()) succeed during that window. - Why this is likely a bug: The expected behavior for this test is that external acceptance is deferred until replication viability is known, or that temporary accepts are safely isolated. The current code does neither: it intentionally opens the gate first and then performs replication startup, which allows real client queries in an inconsistent startup state.
Relevant code
server/server.go:194-207
if initializeDefaultDatabase {
err = createDefaultDatabase(ssCfg, gate)
if err != nil {
controller.Stop()
return nil, err
}
// Release the startup gate to accept external connections now that init is finished
gate.Release()
}
// TODO: shutdown replication cleanly when we stop the server
_, err = startReplication(cfg, ssCfg)
if err != nil {
controller.Stop()
return nil, err
}Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**Medium severity — Startup gate opens before replication viability is known**
**What failed:** The gate is released before replication startup is validated, so the process accepts external traffic during a transient window where overall startup is not yet viable.
- **Impact:** When replication startup fails, the server can briefly accept real client queries before shutting down, causing interrupted sessions and unreliable startup behavior for database users.
- **Steps to reproduce:**
1. Start the server with an empty data directory so first-run default database initialization runs under the startup gate.
2. Configure replication to a non-routable upstream (for example `10.255.255.1`) so `startReplication()` cannot establish viability promptly.
3. Race external `psql` connections immediately after startup and observe successful queries before replication startup later fails/stalls.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** In `server/server.go`, the PR-modified startup path calls `createDefaultDatabase(ssCfg, gate)`, then `gate.Release()`, and only afterward runs `startReplication(cfg, ssCfg)`. If replication initialization blocks or fails, external connections have already been allowed. Runtime evidence matches this ordering: logs show `Starting replication` followed by external `NewConnection` IDs 2-8, and captured `psql` queries (`SELECT 1`, `SELECT version()`) succeed during that window.
- **Why this is likely a bug:** The expected behavior for this test is that external acceptance is deferred until replication viability is known, or that temporary accepts are safely isolated. The current code does neither: it intentionally opens the gate first and then performs replication startup, which allows real client queries in an inconsistent startup state.
**Relevant code:**
`server/server.go:194-207`
~~~go
if initializeDefaultDatabase {
err = createDefaultDatabase(ssCfg, gate)
if err != nil {
controller.Stop()
return nil, err
}
// Release the startup gate to accept external connections now that init is finished
gate.Release()
}
// TODO: shutdown replication cleanly when we stop the server
_, err = startReplication(cfg, ssCfg)
if err != nil {
controller.Stop()
return nil, err
}
~~~
|

Dolt's CREATE DATABASE is non-transactional and non-atomic, which means that a different session might see a partially initialized database. This is a much bigger problem for Doltgres because it creates a
postgresdatabase on first run if no database exists, and clients polling for readiness might see this database before it's fully initialized.To fix this, we create the initial database as a startup service, before we begin accepting connections.
Additionally, we add more defensive readiness checks in go-sql-driver, where this race was most commonly observed. They should be redundant at this point.
Depends on dolthub/dolt#11286