[tianzhou]

Summary

The PG14 job of the v1.13.3 release run (38778778375) failed in TestApplyCommand_LockTimeoutRetryNotAppliedToConcurrentIndex with:

waiting for server to start.... stopped waiting
pg_ctl: could not start server

The other four version jobs passed, and the same workflow passed for v1.13.2 ten hours earlier, so this is a flaky launch, not a logic failure.

Cause

findAvailablePort reserves a port by listening on :0 and closing the socket; the embedded-postgres library probes the same port once more, then runs pg_ctl. Nothing holds the port in between, so another process (five test packages start embedded instances in parallel under go test ./...) can take it before the postmaster binds. The server ran with log_min_messages=PANIC, which hides the postmaster's own bind failure (LOG: could not bind ..., FATAL: no socket created for listening). That is why the CI output is silent between "waiting for server to start" and "stopped waiting": any post-config startup failure was swallowed at that level.

Fix (internal/postgres/embedded.go)

• StartEmbeddedPostgres retries the launch up to 3 times, each on a fresh port and runtime directory.
• The data directory of a failed attempt is removed instead of leaking under the temp dir.
• The server runs at log_min_messages=LOG, so a failed launch reports its cause in the error. Output still goes to the library's temp log file (it is passed io.Discard as logger), so nothing new reaches the user's terminal.
• findAvailablePort becomes a package variable so a test can inject a taken port.

Test

TestStartEmbeddedPostgres_RetriesWhenPortIsTaken hands the first attempt a port held by a listener and asserts the second attempt comes up on a different port with exactly one retry. A throwaway variant where every attempt gets the taken port fails after 3 attempts, so the test is not vacuous.

Test plan

• go test ./internal/postgres on PG 18 (default) and PGSCHEMA_POSTGRES_VERSION=14
• go test ./cmd/apply (full package, includes the test that failed in CI)
• go vet ./...
• Confirmed with a scratch program against embedded-postgres directly that a post-config startup failure prints nothing at PANIC and the full cause at LOG

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The changes affect embedded PostgreSQL startup used by plan generation and test setup.

The PR appears safe to merge, with a non-blocking request to test the retry limit.

Findings

1.  Retry limit is untested ▶

Summary

Adds up to three embedded PostgreSQL launch attempts, each with a fresh port and runtime path.

• Embedded PostgreSQL startup retries failed launches on fresh ports.

_{Reviews (1) · Last reviewed commit: "fix: retry embedded PostgreSQL launch wh..." · Reviewed by Greptile}

[Copilot]

🟡 Changes recommended

Failed-start cleanup can delete a still-running PostgreSQL server's data directory without first ensuring process termination.

1 open issue

• Terminate timed-out Postgres before deleting runtime directory · New

What changed in this PR

This PR makes embedded PostgreSQL startup more resilient to port-allocation races during planning and parallel tests.

Changes:

• Retry startup up to three times with fresh ports and runtime directories.
• Remove failed-attempt directories and retain startup diagnostics.
• Add test hooks and a regression test for retrying an occupied port.

File	Description
internal/​postgres/​export_test.go	Exposes internal helpers for testing.
internal/​postgres/​embedded.go	Adds bounded startup retries, cleanup, and diagnostic logging.
internal/​postgres/​embedded_test.go	Tests successful retry after an occupied-port failure.

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

The localized changes have targeted regression coverage and no identified blocking issues; tests were not executed during this review.

No open issues 🎉

1 resolved since last review

• Terminate timed-out Postgres before deleting runtime directory

_{Review effort: Balanced}