WorkLockManager: Reconnect if we are on a read-only replica

[kensimon]

Currently WorkLockManager health checks the connection by running db.ping(), but this will still appear successful even if we're connected to a read-only replica (which may happen if a failover occurs.) Improve the error handling to detect this and explicitly reconnect.

Related issues

nvbugs/6097302

Type of Change

• Add - New feature or capability
• Change - Changes in existing functionality
• Fix - Bug fixes
• Remove - Removed features or deprecated functionality
• Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

• This PR contains breaking changes

Testing

• Unit tests added/updated
• Integration tests added/updated
• Manual testing performed
• No testing required (docs, internal refactor, etc.)

Additional Notes

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f776649a-f830-45b6-924c-ebcb3265970c

📥 Commits

Reviewing files that changed from the base of the PR and between e7a297c and e8fbdda.

📒 Files selected for processing (1)

• crates/api-db/src/work_lock_manager.rs

---

Summary by CodeRabbit

• Bug Fixes
  • Improved lock handling to automatically recover from unhealthy database sessions.
  • Read-only database connections are now detected and replaced before lock operations continue.
  • Keepalive and release actions now work more reliably after a connection swap.

Walkthrough

WorkLockManager's connection handling was reworked to introduce a ReservedConnection wrapper that tracks an optional pooled PgConnection, checks health via transaction_read_only, and reacquires a fresh connection when unhealthy or read-only, replacing prior ping-based logic. Handlers and a new integration test were updated accordingly.

Changes

ReservedConnection health-check rework

Layer / File(s)	Summary
Import adjustments crates/api-db/src/work_lock_manager.rs	Replaces generic Connection import with explicit PgConnection, PgPool, Postgres imports.
ReservedConnection wrapper and health checks crates/api-db/src/work_lock_manager.rs	Adds ReservedConnection with get_if_healthy, reacquire, and connection_is_healthy methods checking transaction_read_only to detect and replace unhealthy/read-only connections.
run_loop and handler wiring crates/api-db/src/work_lock_manager.rs	Reworks run_loop to hold state via ReservedConnection, routes AcquireLock through health checks, and updates ReleaseLock/KeepLockAlive handlers to use the wrapper-provided connection reference.
Integration test for read-only replacement crates/api-db/src/work_lock_manager.rs	Adds PgPoolOptions import and test_read_only_connection_is_replaced, which forces a single-connection pool read-only and asserts the manager replaces it before lock acquisition succeeds.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RunLoop
  participant ReservedConnection
  participant PgPool
  participant Postgres

  RunLoop->>ReservedConnection: get_if_healthy()
  ReservedConnection->>Postgres: transaction_read_only query
  Postgres-->>ReservedConnection: health status
  alt connection unhealthy or read-only
    ReservedConnection->>ReservedConnection: close reserved connection
    RunLoop->>ReservedConnection: reacquire(&pool)
    ReservedConnection->>PgPool: acquire()
    PgPool-->>ReservedConnection: new PoolConnection
    ReservedConnection->>Postgres: transaction_read_only check
    Postgres-->>ReservedConnection: health status
  end
  ReservedConnection-->>RunLoop: PgConnection reference
  RunLoop->>Postgres: execute AcquireLock/ReleaseLock/KeepLockAlive

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: reconnecting WorkLockManager when the database session is read-only.
Description check	✅ Passed	The description directly matches the changeset by explaining the ping check weakness, failover scenario, and reconnect fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Container Scan Summary

Service	Total	Critical	High	Medium	Low	Other
boot-artifacts-aarch64	3	0	0	3	0	0
boot-artifacts-x86_64	3	0	0	3	0	0
forge-admin-cli-x86_64	271	13	34	91	7	126
machine-validation-runner	800	37	234	291	43	195
machine_validation	800	37	234	291	43	195
machine_validation-aarch64	800	37	234	291	43	195
nvmetal-carbide	800	37	234	291	43	195
TOTAL	3477	161	970	1261	179	906

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.