Bookhoard/bookhoard
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 12 Wiki Activity

chore: remove obsolete REFINED_SCHEMA_PLAN.md

Browse Source 
Delete old schema plan file that is no longer needed.
This commit is contained in:
john-okeefe
2026-02-10 20:51:36 -05:00
parent 778d611cab
commit 0eee34ad01
1 changed files with 0 additions and 763 deletions
Download Patch File Download Diff File Expand all files Collapse all files
REFINED_SCHEMA_PLAN.md
-763
View File
@@ -1,763 +0,0 @@
# Refined Implementation Plan: Idempotent Database Schema Initialization
## 🎯 Executive Summary
**Problem:** Application starts before Docker init scripts complete, causing race conditions where `library_types` table doesn't exist when the app tries to create libraries.
**Solution:** Make entire schema.sql idempotent and run it on every application startup with:
- PostgreSQL advisory locking (prevents concurrent execution)
- Single atomic transaction (all-or-nothing execution)
- Post-execution verification (confirms all tables exist)
- Tiered error logging (DEBUG → ERROR → FATAL summary)
**Decision:** Single-file approach following Go/PostgreSQL best practices for maximum maintainability and performance.
---
## 📊 Technical Decisions
### PostgreSQL Version Requirements
**Minimum: PostgreSQL 13** (for `gen_random_uuid()`)
**Recommended: PostgreSQL 15** (matches `docker-compose.yml`)
**IF NOT EXISTS syntax:** PostgreSQL 9.5+ (already satisfied by minimum version)
**Rationale:**
- PostgreSQL 13 introduced `gen_random_uuid()` as a built-in function
- PostgreSQL 15 is the current stable release and matches our deployment target
- All schema features (IF NOT EXISTS, CREATE OR REPLACE FUNCTION) work with PostgreSQL 9.5+
### 1. File Structure: Single Source of Truth
**Decision:** Single `internal/database/schema.go` file referencing existing `database/schema/schema.sql`.
**Rationale:**
- **No duplication**: Single schema file remains the source of truth
- **Clear separation**: `/database/` for schema definitions, `/internal/database/` for Go logic
- **Maintainability**: Only one schema file to maintain
- **Follows existing patterns**: Database definitions stay where they belong
### 2. Function Location: All in One Package
**Decision:** Public `Initialize()` function with private helper functions in same file.
**Rationale:**
- Clean API surface (single public function)
- Related logic co-located (parseTableNames, verifyTables)
- Easy testing with single cohesive unit
- Follows service layer pattern
### 3. Database Type: Concrete Type
**Decision:** Use `*pgxpool.Pool` directly instead of DBTX interface.
**Rationale:**
- **Industry standard:** pgx v5 documentation consistently uses concrete types
- **Performance:** No interface overhead for hot path
- **Simplicity:** No need to define interface for single implementation
- **Testing:** Easy to mock with concrete type
**Reference:** Compare with `sqlc`, `gorm`, `psql` - all use concrete types.
### 4. Lock ID: Pre-computed Constant
**Decision:** Pre-computed FNV-1a hash constant.
**Rationale:**
- **Performance:** Zero runtime overhead vs `generateLockID()` function call
- **Reliability:** Same value every execution (deterministic)
- **Debugging:** Easy to search logs for specific ID value
- **Testing:** Can unit test hash function separately if needed
### 5. Regex Implementation: Stream Scanning
**Decision:** Line-by-line stream scanning with regex pattern matching.
**Rationale:**
- **Performance:** 2x faster for 960-line file (50ms vs 100ms)
- **Memory:** Only current line in memory at once
- **Maintainability:** Simple `bufio.Scanner` pattern
- **Debugging:** Can log which line contains which table
- **Defensive:** Handles both `IF NOT EXISTS` and legacy table statements
**Pattern:** `regexp.MustCompile(`CREATE TABLE (?:IF NOT EXISTS )?(?:\w+\.)?(\w+)`)`
**Note:** Pattern matches both CREATE TABLE formats to support Phase 1 transition and prevent silent skips if tables lack IF NOT EXISTS clause.
---
## 📁 Implementation File Structure
```
internal/database/
├── schema.go # Main initialization logic
├── db.go # Existing sqlc-generated code
├── models.go # Existing sqlc-generated models
├── queries.sql.go # Existing sqlc-generated queries
└── ...
database/
└── schema/
└── schema.sql # Existing single source of truth schema
```
**Single Responsibility:** `schema.go` handles all database initialization logic, referencing the existing schema file.
---
## 🔧 Implementation Plan
### Phase 1: Schema File Verification
#### Step 1.1: Ensure Schema is Idempotent
```bash
# Convert remaining statements to idempotent form
# Current state: 9/27 tables, 20/82 indexes, 1/3 inserts are idempotent
# Tables needing IF NOT EXISTS (18):
# - library_types, users, system_settings, refresh_tokens, libraries
# - library_folders, library_visibility, media_items, reading_progress
# - media_ratings, media_notes, media_highlights, devices, sync_queue
# - sync_conflicts, kobo_shelves, kobo_entitlements, reading_history
# Indexes: Convert remaining 62 CREATE INDEX to CREATE INDEX IF NOT EXISTS
# Inserts needing ON CONFLICT (2):
# - Line 14: library_types (name)
# - Line 44: system_settings (setting_key)
# Already idempotent (no changes needed):
# - 8 ALTER TABLE statements (all have IF NOT EXISTS)
# - 6 CREATE FUNCTION statements (all use OR REPLACE)
# - 1 INSERT statement (system_config has ON CONFLICT)
```
#### Step 1.2: Verify Schema Syntax
```bash
# Test schema.sql syntax validity
psql -h localhost -U postgres -d postgres -f database/schema/schema.sql --echo-errors --quiet
```
#### Step 1.3: Update Local Database
```bash
# CRITICAL: This is pre-production, update local DB after schema.sql changes
podman compose down -v # WARNING: loses all data
podman compose up -d
```
---
### Phase 2: Create Schema Initialization Logic
#### Step 2.1: Create `internal/database/schema.go`
**Implementation Strategy:**
```go
package database
import (
"bufio"
"context"
"embed"
"fmt"
"log"
"regexp"
"strings"
"time"
_ "embed" // Used for go:embed
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgxpool"
)
//go:embed ../../database/schema/schema.sql
var SchemaFile string
const (
// Pre-computed FNV-1a hash of "bookhoard:schema:init"
// Generated using: generateLockID("bookhoard:schema:init")
schemaInitLockID = 7804706162000639061
)
// Hash calculation function (for reference/testing)
func generateLockID(key string) int64 {
hash := uint64(14695981039346656037) // FNV offset basis
for _, c := range key {
hash ^= uint64(c)
hash *= 1099511628211 // FNV prime
}
return int64(hash)
}
// parseTableNames extracts all table names from CREATE TABLE statements
// Uses stream scanning for performance and memory efficiency
// Handles both "CREATE TABLE" and "CREATE TABLE IF NOT EXISTS" formats
func parseTableNames() ([]string, error) {
scanner := bufio.NewScanner(strings.NewReader(SchemaFile))
pattern := regexp.MustCompile(`CREATE TABLE (?:IF NOT EXISTS )?(?:\w+\.)?(\w+)`)
tables := make(map[string]bool)
for scanner.Scan() {
line := scanner.Text()
matches := pattern.FindStringSubmatch(line)
if len(matches) > 1 {
tableName := matches[1]
tables[tableName] = true
}
}
if err := scanner.Err(); err != nil {
return nil, fmt.Errorf("failed to scan schema.sql: %w", err)
}
// Convert map to unique slice
result := make([]string, 0, len(tables))
for table := range tables {
result = append(result, table)
}
return result, nil
}
// verifyTables checks all expected tables exist in database
func verifyTables(ctx context.Context, db *pgxpool.Pool, expectedTables []string) error {
rows, err := db.Query(ctx, `
SELECT table_name
FROM information_schema.tables
WHERE table_schema = 'public'
AND table_type = 'BASE TABLE'
`)
if err != nil {
return fmt.Errorf("failed to query existing tables: %w", err)
}
defer rows.Close()
existingTables := make(map[string]bool)
for rows.Next() {
var tableName string
if err := rows.Scan(&tableName); err != nil {
return fmt.Errorf("failed to scan table name: %w", err)
}
existingTables[tableName] = true
}
if err := rows.Err(); err != nil {
return fmt.Errorf("error iterating tables: %w", err)
}
// Check all expected tables exist
var missing []string
for _, expected := range expectedTables {
if !existingTables[expected] {
missing = append(missing, expected)
}
}
if len(missing) > 0 {
return fmt.Errorf("missing tables: %s", strings.Join(missing, ", "))
}
return nil
}
// verifyFunctions checks all expected functions exist in database
func verifyFunctions(ctx context.Context, db *pgxpool.Pool, expectedFunctions []string) error {
rows, err := db.Query(ctx, `
SELECT routine_name
FROM information_schema.routines
WHERE routine_schema = 'public'
AND routine_type = 'FUNCTION'
`)
if err != nil {
return fmt.Errorf("failed to query existing functions: %w", err)
}
defer rows.Close()
existingFunctions := make(map[string]bool)
for rows.Next() {
var functionName string
if err := rows.Scan(&functionName); err != nil {
return fmt.Errorf("failed to scan function name: %w", err)
}
existingFunctions[functionName] = true
}
if err := rows.Err(); err != nil {
return fmt.Errorf("error iterating functions: %w", err)
}
// Check all expected functions exist
var missing []string
for _, expected := range expectedFunctions {
if !existingFunctions[expected] {
missing = append(missing, expected)
}
}
if len(missing) > 0 {
return fmt.Errorf("missing functions: %s", strings.Join(missing, ", "))
}
return nil
}
// executeSchema runs the entire schema.sql in a single transaction
func executeSchema(ctx context.Context, db *pgxpool.Pool) error {
tx, err := db.Begin(ctx)
if err != nil {
return fmt.Errorf("failed to start transaction: %w", err)
}
defer tx.Rollback(ctx)
_, err = tx.Exec(ctx, SchemaFile)
if err != nil {
log.Printf("HINT: Run manually to debug: psql -h localhost -U postgres -d bookhoard -f database/schema/schema.sql")
return fmt.Errorf("schema execution failed: %w", err)
}
if err := tx.Commit(ctx); err != nil {
return fmt.Errorf("failed to commit schema transaction: %w", err)
}
return nil
}
// Initialize ensures that database schema is up-to-date
// Main entry point - runs idempotently on every startup with paranoid verification
func Initialize(ctx context.Context, db *pgxpool.Pool) error {
log.Println("🔐 Acquiring PostgreSQL advisory lock for schema initialization...")
// Get database connection
conn, err := db.Acquire(ctx)
if err != nil {
return fmt.Errorf("failed to acquire database connection: %w", err)
}
defer conn.Release()
// Get advisory lock with timeout (prevents indefinite hangs)
log.Printf("DEBUG: Attempting to acquire advisory lock %d...", schemaInitLockID)
lockCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()
var lockAcquired bool
err = conn.QueryRow(lockCtx, "SELECT pg_try_advisory_lock($1)", schemaInitLockID).Scan(&lockAcquired)
if err != nil {
return fmt.Errorf("failed to acquire advisory lock: %w", err)
}
if !lockAcquired {
log.Println("⏳ Another instance is initializing schema, waiting...")
// Use pg_advisory_lock instead (blocks until available or timeout)
_, err = conn.Exec(lockCtx, "SELECT pg_advisory_lock($1)", schemaInitLockID)
if err != nil {
return fmt.Errorf("timeout waiting for advisory lock: %w", err)
}
}
log.Println("✅ Advisory lock acquired")
defer func() {
// Release lock when done
_, err = conn.Exec(ctx, "SELECT pg_advisory_unlock($1)", schemaInitLockID)
if err != nil {
log.Printf("WARNING: Failed to release advisory lock: %v", err)
} else {
log.Println("🔓 Advisory lock released")
}
}()
// Parse schema.sql to extract expected table names
log.Println("📋 Parsing schema.sql for expected tables...")
expectedTables, err := parseTableNames()
if err != nil {
return fmt.Errorf("failed to parse schema.sql: %w", err)
}
log.Printf("DEBUG: Found %d expected tables in schema.sql", len(expectedTables))
// Critical functions that must exist for app to function
criticalFunctions := []string{
"update_updated_at_column",
"detect_format_group",
"convert_progress",
"detect_conflict",
"merge_progress",
"bulk_update_progress_from_koreader",
}
// Execute schema in a single transaction
log.Println("🔧 Executing schema.sql in transaction...")
err = executeSchema(ctx, db)
if err != nil {
log.Printf("ERROR: Schema execution failed: %v", err)
return fmt.Errorf("schema execution failed: %w", err)
}
log.Println("✅ Schema executed successfully")
// Verify all expected tables exist
log.Println("🔍 Verifying all expected tables exist...")
err = verifyTables(ctx, db, expectedTables)
if err != nil {
log.Printf("ERROR: Schema verification failed: %v", err)
return fmt.Errorf("schema verification failed: %w", err)
}
log.Println("✅ All expected tables verified")
// Verify critical functions exist
log.Println("🔍 Verifying critical functions exist...")
err = verifyFunctions(ctx, db, criticalFunctions)
if err != nil {
log.Printf("ERROR: Function verification failed: %v", err)
return fmt.Errorf("function verification failed: %w", err)
}
log.Println("✅ All critical functions verified")
log.Printf("✅ Database schema initialization complete (%d tables, %d functions verified)",
len(expectedTables), len(criticalFunctions))
return nil
}
```
#### Step 2.2: Verify Embed Path
```bash
# Test that the embed path resolves correctly
cd /home/nymusicman/Code/bookhoard/internal/database
go test -c -o /tmp/test_embed .
# Or simply verify the relative path exists:
ls -la ../../database/schema/schema.sql
# Should show the file exists at the correct relative location
```
#### Step 2.3: Add Unit Tests for schema.go
Create `internal/database/schema_test.go`:
```go
package database
import (
"context"
"testing"
"github.com/jackc/pgx/v5/pgxpool"
"github.com/stretchr/testify/assert"
)
func TestParseTableNames(t *testing.T) {
tables, err := parseTableNames()
assert.NoError(t, err)
assert.Greater(t, len(tables), 20, "Should parse at least 20 tables")
// Verify critical tables are detected
tableMap := make(map[string]bool)
for _, table := range tables {
tableMap[table] = true
}
assert.True(t, tableMap["users"], "Should find 'users' table")
assert.True(t, tableMap["libraries"], "Should find 'libraries' table")
assert.True(t, tableMap["media_items"], "Should find 'media_items' table")
}
func TestGenerateLockID(t *testing.T) {
result := generateLockID("bookhoard:schema:init")
assert.Equal(t, int64(7804706162000639061), result, "FNV-1a hash must match expected value")
}
func TestVerifyTables_MissingTables(t *testing.T) {
// This would require a mock database or test setup
// For now, skip or implement with pgxpool mock
t.Skip("Requires database connection - implement in integration tests")
}
```
Run tests with:
```bash
go test ./internal/database -v
```
---
### Phase 3: Integrate into main.go
#### Step 3.1: Add Schema Initialization to main.go
**Location:** After database connection, before handler creation
**Code to Add:**
```go
import (
// ... existing imports ...
"bookhoard/internal/database"
)
func main() {
// ... existing config loading ...
dbPool, err := pgxpool.New(context.Background(), cfg.DatabaseURL())
if err != nil {
log.Fatal("Failed to connect to database:", err)
}
defer dbPool.Close()
queries := database.New(dbPool)
// ===== NEW: Schema Initialization =====
log.Println("🔧 Ensuring database schema is initialized...")
ctx := context.Background()
if err := database.Initialize(ctx, dbPool); err != nil {
log.Fatal("❌ Database schema initialization failed:", err)
}
log.Println("✅ Database schema initialized and verified, starting server...")
// ===== END NEW =====
// ... continue with existing startup code ...
}
```
---
## 📋 Implementation Verification Checklist
### Before Declaring Complete
**Phase 1 Verification:**
- [ ] Converted 18 CREATE TABLE → CREATE TABLE IF NOT EXISTS (27 total)
- [ ] Converted 62 CREATE INDEX → CREATE INDEX IF NOT EXISTS (82 total)
- [ ] Added ON CONFLICT to 2 INSERT statements (3 total)
- [ ] Verified 8 ALTER TABLE statements have IF NOT EXISTS (already done)
- [ ] Verified 6 CREATE FUNCTION statements use OR REPLACE (already done)
- [ ] Verified schema.sql syntax is valid
- [ ] Confirmed single schema.sql exists as source of truth
**Phase 2 Verification:**
- [ ] `internal/database/schema.go` compiles without errors
- [ ] Single file with all schema initialization functions
- [ ] Imports only what's used (bufio, context, embed, fmt, log, regexp, strings, time, pgx v5)
- [ ] Uses *pgxpool.Pool concrete type
- [ ] Pre-computed constant for lock ID (7804706162000639061)
- [ ] Stream scanning implementation for performance
- [ ] Regex pattern handles both IF NOT EXISTS and legacy table formats
- [ ] Lock acquisition has 30-second timeout to prevent indefinite hangs
- [ ] Successfully embeds existing schema.sql file
- [ ] No schema file duplication (single source of truth)
- [ ] Verifies critical functions exist, not just tables
- [ ] Provides clear error messages for missing schema components
- [ ] Includes debug hint when schema execution fails (psql command)
**Phase 3 Verification:**
- [ ] `cmd/server/main.go` compiles without errors
- [ ] Schema initialization call added before handler creation
- [ ] Proper error handling with Fatal on failure
- [ ] Uses correct import path (`database.Initialize`)
**Integration Testing:**
- [ ] Fresh database initializes correctly
- [ ] Existing database doesn't break
- [ ] Concurrent startup works with advisory locks (manual test below)
- [ ] Partial state recovers successfully
- [ ] TestKoboInitialization passes
- [ ] All integration tests pass
- [ ] Schema initialization messages appear in logs
- [ ] Error messages are clear and actionable
- [ ] No regressions in existing functionality
- [ ] Documentation is complete and accurate
- [ ] `go build ./...` succeeds for entire project
- [ ] `bash scripts/verify-guidelines.sh` passes (0 errors)
**Concurrent Startup Test (Manual):**
```bash
# Test advisory locks work correctly with multiple instances
# Start 5 instances simultaneously
for i in {1..5}; do
podman compose -p test$i up -d app
sleep 0.5 # Stagger starts slightly
done
# Wait 10 seconds for all to initialize
sleep 10
# Check all logs for successful initialization
for i in {1..5}; do
echo "=== Instance $i ==="
podman logs test$i_bookhoard_app 2>&1 | grep "schema initialization complete"
done
# Cleanup
for i in {1..5}; do
podman compose -p test$i down
done
```
**Expected result:** All 5 instances should show "schema initialization complete" in logs, proving advisory locks prevent race conditions.
---
## 📈 Expected Outcomes
### What Will Work:
✅ **App starts successfully** on fresh database
✅ **App starts successfully** on existing database
✅ **Multiple instances can start** simultaneously with advisory locks (30s timeout)
✅ **TestKoboInitialization and all tests pass**
✅ **Clear log messages** show schema initialization with table/function counts
✅ **Production-safe** with no race conditions
✅ **Self-healing** from partial/corrupted state (missing tables or functions)
✅ **Better error messages** indicate which schema components are missing
### What Will NOT Change:
✅ **No changes to API endpoints**
✅ **No changes to database structure** (only safety clauses added)
✅ **No changes to existing data**
✅ **No changes to business logic**
✅ **Backward compatible** with existing deployments
### Performance Impact:
- **Fresh database startup:** +500ms (one-time cost)
- **Existing database startup:** +100ms (verification only)
- **HTTP request handling:** No change
- **Database queries:** No change
- **Memory usage:** Minimal increase (schema parsing optimization)
- **Lock timeout:** 30-second safety prevents indefinite hangs
---
## 🔄 Rollback Plan
If critical errors occur:
### Option 1: Restore Schema File
```bash
git checkout HEAD -- internal/database/schema.go
```
### Option 2: Restore Integration
```bash
git checkout HEAD -- cmd/server/main.go
```
### Option 4: Full Rollback
```bash
git checkout backup-before-schema-idempotent -- .
```
---
## 🚀 Deployment Notes
### First Deployment (Production)
**No manual database setup required.**
The app will initialize the database automatically on first startup.
**Monitor logs for:**
```
✅ Database schema initialization complete (27 tables, 6 functions verified)
```
### Subsequent Deployments
**Schema changes apply automatically.**
New tables/columns added to schema.sql will be created on next startup.
### Rollback Plan
If new version has schema issues:
1. Deploy previous version
2. Previous version will verify and use existing schema
3. No data loss (CREATE IF NOT EXISTS preserves data)
---
## 📞 Support
### PostgreSQL Version Issues
If you see errors about `gen_random_uuid()` not existing:
```bash
# Check PostgreSQL version
podman exec bookhoard_db psql --version
# Should be 13.0 or higher
```
If using an older PostgreSQL version, update `docker-compose.yml`:
```yaml
db:
image: postgres:15-alpine # Or postgres:13-alpine minimum
```
### Troubleshooting
If issues occur:
### Check logs first:
```bash
podman logs bookhoard_app | grep -i schema
```
### Verify tables exist:
```bash
podman exec bookhoard_db psql -U postgres -d bookhoard -c "\dt"
```
### Verify functions exist:
```bash
podman exec bookhoard_db psql -U postgres -d bookhoard -c "
SELECT routine_name FROM information_schema.routines
WHERE routine_schema = 'public' AND routine_type = 'FUNCTION';
"
```
### Check advisory locks:
```bash
podman exec bookhoard_db psql -U postgres -d bookhoard -c "
SELECT * FROM pg_locks WHERE objid = 7804706162000639061;
"
```
### Force re-initialization (if needed):
```bash
podman compose down -v # WARNING: Deletes all data
podman compose up -d
```
---
## 📚 Documentation
Create documentation explaining the automatic schema initialization:
- **docs/contributing/database-schema.md** - How schema initialization works, how to modify schema safely, PostgreSQL version requirements, future schema changes guidance
- **README.md** - Add "Database Initialization" section documenting first-run behavior and PostgreSQL version requirements
- **internal/database/schema_test.go** - Unit tests for schema parsing and verification functions
### Future Schema Changes
This plan handles **initialization** only (ensuring database is ready for first run or existing deployments). For future schema modifications:
**Safe changes (additive, handled by this plan):**
- ✅ Add new tables: `CREATE TABLE IF NOT EXISTS` - handled automatically
- ✅ Add new columns: `ALTER TABLE ... ADD COLUMN IF NOT EXISTS` - handled automatically
- ✅ Add new indexes: `CREATE INDEX IF NOT EXISTS` - handled automatically
- ✅ Add new functions: `CREATE OR REPLACE FUNCTION` - handled automatically
- ✅ Insert default data: Use `ON CONFLICT DO NOTHING` - handled automatically
**Breaking changes (require manual migration planning):**
- ⚠️ Drop columns: Requires manual migration script
- ⚠️ Rename tables/columns: Requires coordinated deployment with code changes
- ⚠️ Change column types: Requires data migration and potential downtime
- ⚠️ Modify constraints: Requires careful planning and testing
- ⚠️ Remove functions: Ensure no dependencies exist before removal
**Migration strategy for breaking changes:**
1. Create dedicated migration script in `database/migrations/`
2. Version the migration (e.g., `001_drop_legacy_column.sql`)
3. Test migration on backup database first
4. Deploy with application code that handles both old and new schema
5. Run migration during maintenance window or use online schema change tools
6. Verify application compatibility after migration
**Best practice:** Prefer additive changes over breaking changes whenever possible to leverage automatic initialization.
---
**End of Refined Implementation Plan**