60c801c0c2## Summary Fixes race conditions in concurrent server publishing that could result in data integrity violations: - Multiple versions marked as `isLatest=true` for the same server - Duplicate (serverId, version) combinations - Same server name assigned different serverIDs ## Solution Uses two complementary approaches for correctness: ### 1. PostgreSQL Advisory Locks - Serializes concurrent publishes per server name using `pg_advisory_xact_lock` - Uses FNV-1a hash to convert server name to lock ID - Transaction-scoped locks auto-release on commit/rollback - **Required after #465**: The serverId introduction means concurrent publishes must atomically coordinate whether to reuse an existing serverId (for additional versions) or generate a new one (for first version). Without locks, race conditions would assign different serverIDs to versions of the same server. ### 2. Unique Constraints (Migration 007) Two new indexes provide data integrity guarantees: - **idx_unique_server_version**: Ensures (serverId, version) is unique across all versions - **idx_unique_latest_version**: Partial index ensuring only one version per serverId has `isLatest=true` These constraints serve as defense-in-depth safety nets. Even if lock logic had a bug, the database would reject integrity violations. Also drops redundant `idx_servers_version_id` since `version_id` is already the primary key. ## Why WithPublishLock validation is needed after #465 The `ctx.Err()` check at the start of `WithPublishLock` is critical because: 1. After #465 introduced serverId, the publish flow must atomically check existing versions and assign/reuse serverId 2. If the context is cancelled during lock acquisition, we could deadlock 3. Checking ctx validity before acquiring locks prevents these deadlocks ## Testing Added `TestPublishConcurrentVersionsNoRace` with 100 concurrent goroutines publishing different versions of the same server. All succeed with: - Exactly one version marked as latest - All versions sharing the same serverId - No duplicate (serverId, version) combinations NB: this only covers the in memory db, not the postgres db. It'd be nice to write tests for this one as well, but our current setup is a bit annoying for this. Separately looking into ways to easily run all our tests with real postgres and ideally scrap memory.go completely.
21 lines
No EOL
895 B
SQL
21 lines
No EOL
895 B
SQL
-- Add unique constraints to prevent race conditions in concurrent publishing
|
|
-- These serve as safety nets alongside advisory locks
|
|
|
|
-- Drop redundant index (version_id is already the primary key)
|
|
DROP INDEX IF EXISTS idx_servers_version_id;
|
|
|
|
-- Prevent duplicate versions of the same server
|
|
-- Ensures (serverId, version) is unique across all server versions
|
|
CREATE UNIQUE INDEX idx_unique_server_version
|
|
ON servers (
|
|
(value->'_meta'->'io.modelcontextprotocol.registry/official'->>'serverId'),
|
|
(value->>'version')
|
|
);
|
|
|
|
-- Prevent multiple versions marked as latest for the same server
|
|
-- Ensures only one version per serverId can have isLatest=true
|
|
CREATE UNIQUE INDEX idx_unique_latest_version
|
|
ON servers (
|
|
(value->'_meta'->'io.modelcontextprotocol.registry/official'->>'serverId')
|
|
)
|
|
WHERE (value->'_meta'->'io.modelcontextprotocol.registry/official'->>'isLatest')::boolean = true; |