fix(publish): validate registries outside the transaction and time the pool wait

[rdimitrov]

Why

Publish latency has been the only alert firing in production — 1,285 times since 2025-09-11, while the read-path rules fired 6 times in total (last one 2025-09-14). Digging into where that time actually goes turned up a contradiction:

Layer	Publishes (7d)	Over 10s	Max
HTTP histogram (/v0/publish)	9,939	17	45.5s observed
Per-phase publish log (createServerInTransaction)	7,845	0	6.5s

validate_ms — the external registry calls — peaks at 3.9s (p50 18ms, p99 1.1s). So the slow requests were spending their time somewhere the phase log could not see.

The log had a blind spot exactly where the problem is. InTransaction calls pool.Begin and then invokes the transaction body, but the timing started inside that body. A publish that waited 40s for a free connection logged total_ms=200 and looked healthy.

What

Time the pool wait. The gap between a timestamp taken before InTransactionT and the first line of its callback is exactly how long pool.Begin blocked. Reported as pool_wait_ms.

Move registry-ownership validation ahead of the transaction. It fans out to npm, PyPI, NuGet, Cargo, OCI and MCPB with a 10s timeout per host. Running it inside the transaction pinned a pgxpool connection for the duration of those round-trips — so under a publish burst (up to 276 in a 4-minute window) connections are held by requests doing no database work at all, starving a pool already sitting at 150 of 200 connections and stalling unrelated requests in pool.Begin.

The check reads none of our own state, so hoisting it introduces no race, and the per-server advisory lock was already acquired after validation.

To keep one log event per publish, timing and logging move up into CreateServer via a shared publishTimings; createServerInTransaction fills in the phases that still run inside.

Note on total_ms

total_ms now covers the whole CreateServer call rather than just the transaction body, so the residual after the named phases is commit plus overhead. Nothing consumes these log fields — all four Grafana rules are built on mcp_registry_* metrics.

Out of scope

The edit path keeps its validation inside the transaction, with a comment explaining why: skipRegistryValidation is derived from a read inside that transaction, so hoisting needs a second read before it opens plus a decision about the resulting race. It carries a fraction of publish traffic — worth revisiting if it shows up in pool_wait_ms.

Testing

New tests in internal/service/registry_service_publish_transaction_test.go, using a small database.Database double to observe the transaction boundary (which a real database cannot):

• validation failure opens no transaction — the invariant this change introduces
• pool_wait_ms covers time pool.Begin blocked — via a double whose InTransaction sleeps
• a validation failure still emits one publish failed naming failed_phase=validate
• a failure raised inside the transaction is still captured by the now-outer deferred logger

Existing tests pass unchanged. go test ./... (12 packages), -race clean on the service package, golangci-lint 0 issues.

Verifying this in production

The payoff is only visible once deployed: pool_wait_ms will either confirm pool starvation as the mechanism or rule it out. If it comes back near zero on the slow publishes, the remaining suspects are JWT validation, body decode, and the per-request JSON schema compile (schema.go:196-210 builds a fresh compiler per call — measured at 676µs and 882KB per call, deliberately not touched here).

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Moves registry validation outside publish transactions and expands latency attribution.

Changes:

• Validates ownership before acquiring a database connection.
• Adds transaction-start timing and structured publish logging.
• Tags and tests publish requests by API version.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
internal/service/registry_service.go	Refactors publish validation, transaction timing, and logging.
internal/service/registry_service_publish_transaction_test.go	Tests transaction boundaries and timing logs.
internal/api/handlers/v0/publish.go	Adds API-version context attribution.
internal/api/handlers/v0/publish_api_version_test.go	Tests API-version logging end to end.

Suppressed comments (1)

internal/service/registry_service.go:211

• A transaction commit failure leaves gotConnection true while no timed callback phase failed, so the deferred event logs publish failed with an empty failed_phase. PostgreSQL.InTransaction can return a commit error after the callback succeeds (internal/database/postgres.go:755-756). Attribute this case explicitly so the new outer logger does not emit an ambiguous failure.

	if !gotConnection {

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.