fix: prevent NULL status values during migration 008 (#587)
## Summary - Fixes migration 008 failing with "column contains null values" error - Adds COALESCE protection to handle 126 servers without status fields - Includes test utilities to reproduce and verify fixes with production data ## Problem Migration 008 was failing in production because: - 126 out of 781 servers have no `status` field in their JSON - When extracting `rec.value->>'status'`, these became NULL values - The `ALTER TABLE servers ALTER COLUMN status SET NOT NULL` constraint then failed ## Solution 1. **Prevent NULLs during migration**: Use COALESCE to provide 'active' default when status is NULL/empty 2. **Enhanced debugging**: Added RAISE NOTICE statements to log problematic values 3. **Safety check**: Double-check that throws exception if any NULLs remain 4. **Test utilities**: Created mirror_data tools to test migrations with real production data ## Testing Created and used `scripts/mirror_data/` tools to: - Fetch all 781 servers from production API - Load them into a test database - Verify the migration fix works correctly ### Results: - **Before fix**: 126 NULL status values created during migration - **After fix**: 0 NULL status values, all servers have valid status - Migration completes successfully with NOT NULL constraint ## Test plan - [x] Tested locally with production data using mirror_data tools - [x] Verified 0 NULL values after migration (vs 126 before) - [x] Confirmed NOT NULL constraint adds successfully - [ ] Deploy to staging and verify migration succeeds - [ ] Monitor production deployment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
e39b008b41
commit
2d49e5b68c
5 changed files with 506 additions and 2 deletions
|
|
@ -38,7 +38,10 @@ BEGIN
|
|||
-- Note: status is at top level in old format, not in official_meta
|
||||
UPDATE servers
|
||||
SET
|
||||
status = rec.value->>'status',
|
||||
status = COALESCE(
|
||||
NULLIF(NULLIF(rec.value->>'status', ''), 'null'),
|
||||
'active'
|
||||
),
|
||||
published_at = COALESCE((official_meta->>'publishedAt')::TIMESTAMP WITH TIME ZONE, NOW()),
|
||||
updated_at = COALESCE((official_meta->>'updatedAt')::TIMESTAMP WITH TIME ZONE, NOW()),
|
||||
is_latest = COALESCE((official_meta->>'isLatest')::BOOLEAN, true)
|
||||
|
|
@ -47,7 +50,10 @@ BEGIN
|
|||
-- Handle records without official metadata (set defaults)
|
||||
UPDATE servers
|
||||
SET
|
||||
status = rec.value->>'status',
|
||||
status = COALESCE(
|
||||
NULLIF(NULLIF(rec.value->>'status', ''), 'null'),
|
||||
'active'
|
||||
),
|
||||
published_at = NOW(),
|
||||
updated_at = NOW(),
|
||||
is_latest = true
|
||||
|
|
@ -84,6 +90,23 @@ DROP FUNCTION migrate_official_metadata();
|
|||
|
||||
-- Safety check: Normalize invalid status values before adding NOT NULL constraints
|
||||
-- This handles: NULL (missing field), empty string, string "null", and any invalid values
|
||||
-- First, log how many records have problematic status values for debugging
|
||||
DO $$
|
||||
DECLARE
|
||||
null_count INTEGER;
|
||||
empty_count INTEGER;
|
||||
invalid_count INTEGER;
|
||||
BEGIN
|
||||
SELECT COUNT(*) INTO null_count FROM servers WHERE status IS NULL;
|
||||
SELECT COUNT(*) INTO empty_count FROM servers WHERE status = '' OR status = 'null';
|
||||
SELECT COUNT(*) INTO invalid_count FROM servers WHERE status IS NOT NULL AND status NOT IN ('active', 'deprecated', 'deleted', '', 'null');
|
||||
|
||||
IF null_count > 0 OR empty_count > 0 OR invalid_count > 0 THEN
|
||||
RAISE NOTICE 'Status cleanup: NULL=%, empty/null string=%, invalid=%', null_count, empty_count, invalid_count;
|
||||
END IF;
|
||||
END $$;
|
||||
|
||||
-- Fix all problematic status values
|
||||
UPDATE servers
|
||||
SET status = 'active'
|
||||
WHERE status IS NULL
|
||||
|
|
@ -91,6 +114,17 @@ WHERE status IS NULL
|
|||
OR status = 'null'
|
||||
OR status NOT IN ('active', 'deprecated', 'deleted');
|
||||
|
||||
-- Double-check that no NULLs remain (this should never fail after the above)
|
||||
DO $$
|
||||
DECLARE
|
||||
remaining_nulls INTEGER;
|
||||
BEGIN
|
||||
SELECT COUNT(*) INTO remaining_nulls FROM servers WHERE status IS NULL;
|
||||
IF remaining_nulls > 0 THEN
|
||||
RAISE EXCEPTION 'Still have % NULL status values after cleanup!', remaining_nulls;
|
||||
END IF;
|
||||
END $$;
|
||||
|
||||
UPDATE servers SET published_at = NOW() WHERE published_at IS NULL;
|
||||
|
||||
-- Safety check: Ensure updated_at is never NULL
|
||||
|
|
|
|||
2
scripts/mirror_data/.gitignore
vendored
Normal file
2
scripts/mirror_data/.gitignore
vendored
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
# Ignore downloaded production data
|
||||
production_servers.json
|
||||
237
scripts/mirror_data/README.md
Normal file
237
scripts/mirror_data/README.md
Normal file
|
|
@ -0,0 +1,237 @@
|
|||
# Mirror Production Data
|
||||
|
||||
> **Note:** These tools were created by Claude Code as a simple way to kick the tires on data migrations.
|
||||
> They are not intended for production use.
|
||||
|
||||
Tools to fetch and load production registry data for local testing and migration debugging.
|
||||
|
||||
## Overview
|
||||
|
||||
These scripts help you:
|
||||
1. Fetch all server data from the production registry API
|
||||
2. Load it into a local PostgreSQL database
|
||||
3. Test migrations against real production data
|
||||
|
||||
## Prerequisites
|
||||
|
||||
- Go 1.24.x
|
||||
- PostgreSQL (via Docker or local installation)
|
||||
- Required Go packages:
|
||||
```bash
|
||||
go get github.com/google/uuid github.com/lib/pq
|
||||
```
|
||||
|
||||
## Usage
|
||||
|
||||
### 1. Fetch Production Data
|
||||
|
||||
```bash
|
||||
# From the project root directory
|
||||
go run scripts/mirror_data/fetch_production_data.go
|
||||
```
|
||||
|
||||
This will:
|
||||
- Fetch all servers from https://registry.modelcontextprotocol.io/v0/servers
|
||||
- Handle pagination automatically
|
||||
- Save data to `scripts/mirror_data/production_servers.json`
|
||||
- Be respectful to the API with rate limiting
|
||||
|
||||
Output: `production_servers.json` containing all server records
|
||||
|
||||
### 2. Set Up Test Database
|
||||
|
||||
Start a PostgreSQL container for testing:
|
||||
|
||||
```bash
|
||||
docker run -d --name test-postgres \
|
||||
-p 5433:5432 \
|
||||
-e POSTGRES_PASSWORD=testpass \
|
||||
-e POSTGRES_DB=registry_test \
|
||||
postgres:16
|
||||
```
|
||||
|
||||
Or reset an existing database:
|
||||
|
||||
```bash
|
||||
docker exec test-postgres psql -U postgres postgres -c "DROP DATABASE IF EXISTS registry_test;"
|
||||
docker exec test-postgres psql -U postgres postgres -c "CREATE DATABASE registry_test;"
|
||||
```
|
||||
|
||||
### 3. Load Production Data
|
||||
|
||||
```bash
|
||||
# From the project root directory
|
||||
go run scripts/mirror_data/load_production_data.go
|
||||
```
|
||||
|
||||
This will:
|
||||
1. Connect to PostgreSQL at `localhost:5433`
|
||||
2. Apply migrations up to a configurable point (default: migration 007)
|
||||
3. Load all production server data
|
||||
4. Analyze the data and report statistics
|
||||
5. Show sample servers with NULL status values
|
||||
|
||||
To test a different migration, edit `maxMigration` in the script (line 24)
|
||||
|
||||
### 4. Test Migrations
|
||||
|
||||
After loading the data, you can test migrations against real production data.
|
||||
|
||||
#### Testing a Single Migration
|
||||
|
||||
```bash
|
||||
# Test migration 008
|
||||
cat internal/database/migrations/008_separate_official_metadata.sql | \
|
||||
docker exec -i test-postgres psql -U postgres -d registry_test
|
||||
```
|
||||
|
||||
#### Debugging Failed Migrations
|
||||
|
||||
1. **Check the error output:**
|
||||
```bash
|
||||
# Run migration and capture all output
|
||||
cat internal/database/migrations/008_separate_official_metadata.sql | \
|
||||
docker exec -i test-postgres psql -U postgres -d registry_test 2>&1 | \
|
||||
grep -E "(ERROR|NOTICE|WARNING)"
|
||||
```
|
||||
|
||||
2. **Inspect data before migration:**
|
||||
```bash
|
||||
# Check for NULL values that might cause issues
|
||||
docker exec test-postgres psql -U postgres -d registry_test -c \
|
||||
"SELECT COUNT(*) FROM servers WHERE value->>'status' IS NULL;"
|
||||
|
||||
# Find sample problematic records
|
||||
docker exec test-postgres psql -U postgres -d registry_test -c \
|
||||
"SELECT value->>'name', value->>'version' FROM servers \
|
||||
WHERE value->>'status' IS NULL LIMIT 5;"
|
||||
```
|
||||
|
||||
3. **Test partial migrations:**
|
||||
```bash
|
||||
# Extract and run only part of a migration to isolate issues
|
||||
# For example, run only up to the NOT NULL constraint:
|
||||
docker exec test-postgres psql -U postgres -d registry_test <<EOF
|
||||
BEGIN;
|
||||
-- Paste migration SQL up to the problematic line
|
||||
-- Check intermediate state
|
||||
SELECT COUNT(*) FROM servers WHERE status IS NULL;
|
||||
ROLLBACK; -- or COMMIT if testing further
|
||||
EOF
|
||||
```
|
||||
|
||||
#### Testing Migration Rollback
|
||||
|
||||
```bash
|
||||
# Start a transaction to test and rollback
|
||||
docker exec test-postgres psql -U postgres -d registry_test <<EOF
|
||||
BEGIN;
|
||||
\i /dev/stdin < internal/database/migrations/008_separate_official_metadata.sql
|
||||
-- Inspect the results
|
||||
\d servers
|
||||
SELECT COUNT(*) FROM servers;
|
||||
ROLLBACK; -- Undo all changes
|
||||
EOF
|
||||
```
|
||||
|
||||
#### Iterative Testing Workflow
|
||||
|
||||
1. **Reset database to clean state:**
|
||||
```bash
|
||||
docker exec test-postgres psql -U postgres postgres -c "DROP DATABASE IF EXISTS registry_test;"
|
||||
docker exec test-postgres psql -U postgres postgres -c "CREATE DATABASE registry_test;"
|
||||
go run scripts/mirror_data/load_production_data.go
|
||||
```
|
||||
|
||||
2. **Test your migration changes:**
|
||||
```bash
|
||||
cat internal/database/migrations/008_separate_official_metadata.sql | \
|
||||
docker exec -i test-postgres psql -U postgres -d registry_test
|
||||
```
|
||||
|
||||
3. **Verify the results:**
|
||||
```bash
|
||||
# Check final table structure
|
||||
docker exec test-postgres psql -U postgres -d registry_test -c "\d servers"
|
||||
|
||||
# Verify data integrity
|
||||
docker exec test-postgres psql -U postgres -d registry_test -c \
|
||||
"SELECT COUNT(*), COUNT(DISTINCT server_name) FROM servers;"
|
||||
```
|
||||
|
||||
#### Common Migration Issues to Check
|
||||
|
||||
- **NULL values:** Check columns before adding NOT NULL constraints
|
||||
- **Unique constraints:** Verify no duplicates exist before adding
|
||||
- **Check constraints:** Validate all existing data matches the constraint
|
||||
- **Data type changes:** Ensure all values can be cast to new type
|
||||
- **Foreign keys:** Verify referential integrity before adding
|
||||
|
||||
### 5. Clean Up
|
||||
|
||||
```bash
|
||||
docker stop test-postgres
|
||||
docker rm test-postgres
|
||||
```
|
||||
|
||||
## Configuration
|
||||
|
||||
The `load_production_data.go` script connects to:
|
||||
- Host: `localhost`
|
||||
- Port: `5433`
|
||||
- Database: `registry_test`
|
||||
- User: `postgres`
|
||||
- Password: `testpass`
|
||||
|
||||
Modify line 17 in the script if you need different connection parameters.
|
||||
|
||||
## Example Output
|
||||
|
||||
When running `load_production_data.go`:
|
||||
|
||||
```
|
||||
Running migrations 001-007...
|
||||
Applying migration 1: 001_initial_schema.sql
|
||||
Applying migration 2: 002_add_server_extensions.sql
|
||||
...
|
||||
|
||||
Loading production data...
|
||||
Loading 779 servers...
|
||||
Data loaded successfully!
|
||||
|
||||
Total servers in database: 779
|
||||
|
||||
Analyzing status field in JSON data...
|
||||
Total servers: 779
|
||||
NULL status: 126
|
||||
Empty status: 0
|
||||
'null' string status: 0
|
||||
'active' status: 652
|
||||
'deprecated' status: 0
|
||||
'deleted' status: 0
|
||||
Other/Invalid: 1
|
||||
|
||||
Sample servers with NULL status:
|
||||
- com.biodnd/agent-ip@0.1.2
|
||||
- io.github.jkakar/cookwith-mcp@1.0.0
|
||||
...
|
||||
|
||||
Database is ready for testing migration 008!
|
||||
```
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### Port already in use
|
||||
If port 5433 is already in use, either:
|
||||
- Stop the existing service using that port
|
||||
- Use a different port in both the Docker command and the Go script
|
||||
|
||||
### Connection refused
|
||||
Ensure the PostgreSQL container is running and healthy:
|
||||
```bash
|
||||
docker ps | grep test-postgres
|
||||
docker logs test-postgres
|
||||
```
|
||||
|
||||
### Migration failures
|
||||
The script stops at migration 007 intentionally. To test migration 008, run it manually after the data is loaded.
|
||||
87
scripts/mirror_data/fetch_production_data.go
Normal file
87
scripts/mirror_data/fetch_production_data.go
Normal file
|
|
@ -0,0 +1,87 @@
|
|||
// This tool was created by Claude Code as a simple way to kick the tires on data migrations
|
||||
// by fetching production data from the public registry API.
|
||||
// It is not intended for production use.
|
||||
//
|
||||
//go:build ignore
|
||||
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"os"
|
||||
"time"
|
||||
)
|
||||
|
||||
type ServerResponse struct {
|
||||
Servers []json.RawMessage `json:"servers"`
|
||||
Metadata struct {
|
||||
NextCursor string `json:"next_cursor,omitempty"`
|
||||
Count int `json:"count"`
|
||||
} `json:"metadata"`
|
||||
}
|
||||
|
||||
func main() {
|
||||
baseURL := "https://registry.modelcontextprotocol.io/v0/servers"
|
||||
var allServers []json.RawMessage
|
||||
cursor := ""
|
||||
pageCount := 0
|
||||
|
||||
for {
|
||||
pageCount++
|
||||
url := baseURL
|
||||
if cursor != "" {
|
||||
url = fmt.Sprintf("%s?cursor=%s", baseURL, cursor)
|
||||
}
|
||||
|
||||
fmt.Printf("Fetching page %d: %s\n", pageCount, url)
|
||||
|
||||
resp, err := http.Get(url)
|
||||
if err != nil {
|
||||
panic(fmt.Sprintf("Failed to fetch: %v", err))
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
panic(fmt.Sprintf("Failed to read body: %v", err))
|
||||
}
|
||||
|
||||
var serverResp ServerResponse
|
||||
if err := json.Unmarshal(body, &serverResp); err != nil {
|
||||
panic(fmt.Sprintf("Failed to parse JSON: %v", err))
|
||||
}
|
||||
|
||||
allServers = append(allServers, serverResp.Servers...)
|
||||
fmt.Printf(" Got %d servers (total: %d)\n", len(serverResp.Servers), len(allServers))
|
||||
|
||||
if serverResp.Metadata.NextCursor == "" {
|
||||
break
|
||||
}
|
||||
cursor = serverResp.Metadata.NextCursor
|
||||
|
||||
// Be nice to the API
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
}
|
||||
|
||||
// Save all servers to a file
|
||||
output := map[string]interface{}{
|
||||
"servers": allServers,
|
||||
"count": len(allServers),
|
||||
"fetched": time.Now().Format(time.RFC3339),
|
||||
}
|
||||
|
||||
data, err := json.MarshalIndent(output, "", " ")
|
||||
if err != nil {
|
||||
panic(fmt.Sprintf("Failed to marshal output: %v", err))
|
||||
}
|
||||
|
||||
outputFile := "production_servers.json"
|
||||
if err := os.WriteFile(outputFile, data, 0644); err != nil {
|
||||
panic(fmt.Sprintf("Failed to write file: %v", err))
|
||||
}
|
||||
|
||||
fmt.Printf("\nDone! Saved %d servers to %s\n", len(allServers), outputFile)
|
||||
}
|
||||
144
scripts/mirror_data/load_production_data.go
Normal file
144
scripts/mirror_data/load_production_data.go
Normal file
|
|
@ -0,0 +1,144 @@
|
|||
// This tool was created by Claude Code as a simple way to kick the tires on data migrations
|
||||
// by loading production data into a test database for migration testing.
|
||||
// It is not intended for production use.
|
||||
//
|
||||
//go:build ignore
|
||||
|
||||
package main
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"log"
|
||||
"path/filepath"
|
||||
|
||||
"github.com/google/uuid"
|
||||
_ "github.com/lib/pq"
|
||||
)
|
||||
|
||||
func main() {
|
||||
// Database connection
|
||||
db, err := sql.Open("postgres", "postgres://postgres:testpass@localhost:5433/registry_test?sslmode=disable")
|
||||
if err != nil {
|
||||
log.Fatal("Failed to connect to database:", err)
|
||||
}
|
||||
defer db.Close()
|
||||
|
||||
// Run migrations up to a specific point (configure as needed)
|
||||
maxMigration := 7 // Change this to test different migration states
|
||||
fmt.Printf("Running migrations 001-%03d...\n", maxMigration)
|
||||
migrationsDir := "internal/database/migrations"
|
||||
for i := 1; i <= maxMigration; i++ {
|
||||
migrationFile := filepath.Join(migrationsDir, fmt.Sprintf("%03d_*.sql", i))
|
||||
files, err := filepath.Glob(migrationFile)
|
||||
if err != nil || len(files) == 0 {
|
||||
log.Fatalf("Migration file %d not found", i)
|
||||
}
|
||||
|
||||
content, err := ioutil.ReadFile(files[0])
|
||||
if err != nil {
|
||||
log.Fatalf("Failed to read migration %d: %v", i, err)
|
||||
}
|
||||
|
||||
fmt.Printf(" Applying migration %d: %s\n", i, filepath.Base(files[0]))
|
||||
if _, err := db.Exec(string(content)); err != nil {
|
||||
log.Fatalf("Failed to apply migration %d: %v", i, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Load production data
|
||||
fmt.Println("\nLoading production data...")
|
||||
data, err := ioutil.ReadFile("scripts/mirror_data/production_servers.json")
|
||||
if err != nil {
|
||||
log.Fatal("Failed to read production data:", err)
|
||||
}
|
||||
|
||||
var prodData struct {
|
||||
Servers []json.RawMessage `json:"servers"`
|
||||
}
|
||||
if err := json.Unmarshal(data, &prodData); err != nil {
|
||||
log.Fatal("Failed to parse production data:", err)
|
||||
}
|
||||
|
||||
fmt.Printf("Loading %d servers...\n", len(prodData.Servers))
|
||||
|
||||
// Prepare insert statement
|
||||
stmt, err := db.Prepare("INSERT INTO servers (version_id, value) VALUES ($1, $2)")
|
||||
if err != nil {
|
||||
log.Fatal("Failed to prepare statement:", err)
|
||||
}
|
||||
defer stmt.Close()
|
||||
|
||||
// Insert each server
|
||||
for i, server := range prodData.Servers {
|
||||
// Generate a unique version_id
|
||||
versionID := uuid.New().String()
|
||||
|
||||
// The server data is already JSON, just insert it
|
||||
if _, err := stmt.Exec(versionID, server); err != nil {
|
||||
log.Printf("Failed to insert server %d: %v", i, err)
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
fmt.Println("Data loaded successfully!")
|
||||
|
||||
// Verify the data
|
||||
var count int
|
||||
db.QueryRow("SELECT COUNT(*) FROM servers").Scan(&count)
|
||||
fmt.Printf("\nTotal servers in database: %d\n", count)
|
||||
|
||||
// Check for NULL status values in the JSON
|
||||
fmt.Println("\nAnalyzing status field in JSON data...")
|
||||
|
||||
rows, err := db.Query(`
|
||||
SELECT
|
||||
COUNT(*) as total,
|
||||
COUNT(CASE WHEN value->>'status' IS NULL THEN 1 END) as null_status,
|
||||
COUNT(CASE WHEN value->>'status' = '' THEN 1 END) as empty_status,
|
||||
COUNT(CASE WHEN value->>'status' = 'null' THEN 1 END) as string_null_status,
|
||||
COUNT(CASE WHEN value->>'status' = 'active' THEN 1 END) as active_status,
|
||||
COUNT(CASE WHEN value->>'status' = 'deprecated' THEN 1 END) as deprecated_status,
|
||||
COUNT(CASE WHEN value->>'status' = 'deleted' THEN 1 END) as deleted_status
|
||||
FROM servers
|
||||
`)
|
||||
if err != nil {
|
||||
log.Fatal("Failed to analyze data:", err)
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
if rows.Next() {
|
||||
var total, nullStatus, emptyStatus, stringNullStatus, activeStatus, deprecatedStatus, deletedStatus int
|
||||
rows.Scan(&total, &nullStatus, &emptyStatus, &stringNullStatus, &activeStatus, &deprecatedStatus, &deletedStatus)
|
||||
|
||||
fmt.Printf(" Total servers: %d\n", total)
|
||||
fmt.Printf(" NULL status: %d\n", nullStatus)
|
||||
fmt.Printf(" Empty status: %d\n", emptyStatus)
|
||||
fmt.Printf(" 'null' string status: %d\n", stringNullStatus)
|
||||
fmt.Printf(" 'active' status: %d\n", activeStatus)
|
||||
fmt.Printf(" 'deprecated' status: %d\n", deprecatedStatus)
|
||||
fmt.Printf(" 'deleted' status: %d\n", deletedStatus)
|
||||
fmt.Printf(" Other/Invalid: %d\n", total-nullStatus-emptyStatus-stringNullStatus-activeStatus-deprecatedStatus-deletedStatus)
|
||||
}
|
||||
|
||||
// Print sample servers with no status
|
||||
fmt.Println("\nSample servers with NULL status:")
|
||||
rows, err = db.Query(`
|
||||
SELECT value->>'name', value->>'version'
|
||||
FROM servers
|
||||
WHERE value->>'status' IS NULL
|
||||
LIMIT 5
|
||||
`)
|
||||
if err == nil {
|
||||
defer rows.Close()
|
||||
for rows.Next() {
|
||||
var name, version string
|
||||
rows.Scan(&name, &version)
|
||||
fmt.Printf(" - %s@%s\n", name, version)
|
||||
}
|
||||
}
|
||||
|
||||
fmt.Printf("\nDatabase is ready for testing migration %03d!\n", maxMigration+1)
|
||||
}
|
||||
Loading…
Reference in a new issue