Database Production Readiness Assessment - Critical Gaps Identified

[vredchenko]

Database Production Readiness Assessment

Executive Summary

Two comprehensive assessments (project-owner and database-admin agents) have identified critical gaps preventing production deployment. Current production readiness: 72% - NOT READY.

Key Verdict: The database schema design is solid (85% complete) but lacks critical operational safeguards required for scientific data with 7-year retention requirements.

Estimated Time to Production: 2-3 weeks of focused work

Critical Blockers (Must Fix Before Production)

1. No Backup/Recovery System (CRITICAL - Priority 1)

Status: No automated backups exist
Impact: Risk of total data loss for scientific data requiring 7-year retention
Effort: 2-3 days

Required Implementation:

• WAL (Write-Ahead Log) archiving for point-in-time recovery
• Daily pg_dump backups with compression
• Restore procedure testing
• Off-site backup storage

Recommendation: See code snippet in comment below for backup script

2. Schema Drift Issue #28 - Migration Regression (CRITICAL - Priority 2)

Status: Schema drift check bypassed in CI with continue-on-error: true
Impact: The "fix schema drift" migration (87f8c5e11906) actually introduced regressions:

• Made status fields nullable (breaks data integrity)
• Removed CASCADE DELETE from foreign keys (creates orphaned records)
• Created more drift instead of fixing it

Effort: 1 day

Required Actions:

1. Roll back migration 87f8c5e11906
2. Create corrected migration with NOT NULL status fields
3. Restore CASCADE DELETE behavior
4. Re-enable schema drift CI check

3. Missing Database Constraints (HIGH - Priority 3)

Status: No unique constraints on natural keys
Impact: Duplicate data entries (evidence: tools/find_foilhole_duplicates.py exists)

Affected Tables:

• foilhole: No unique constraint on (foilhole_id, gridsquare_uuid)
• gridsquare: No unique constraint on (gridsquare_id, grid_uuid)
• micrograph: No unique constraint on (micrograph_id, foilhole_uuid)

Effort: 2 hours + data deduplication time

Risk: Requires deduplicating existing data before adding constraints

4. Insufficient Database Testing (HIGH - Priority 4)

Status: Only 13 test files total, none for database operations
Impact: Cannot verify:

• Constraint violations
• Cascade deletes
• Migration success
• Concurrent access patterns

Effort: 3 days

High Priority Issues (Month 1)

5. Index Inefficiencies

Issues Found:

• Wrong table bug (line 602): Index created on qualitypredictionmodelparameter instead of qualitypredictionmodelweight
• Over-indexing: 147 indexes causing write performance overhead
• Redundant patterns: Both hash and pattern indexes on same columns
• Missing composite indexes for common query patterns

Recommendation: Consolidate to 80-100 strategic indexes

6. Connection Pool Monitoring

Status: No monitoring of pool exhaustion or connection leaks
Current Config: pool_size=10, max_overflow=20 (reasonable but unmonitored)

Required:

• SQLAlchemy event listeners for pool stats
• Alerts on pool exhaustion
• Connection lifecycle tracking

7. PostgreSQL Configuration Tuning

Status: Using defaults, not optimized for scientific workloads

Missing Configuration:

shared_buffers = 8GB           # 25% of RAM
effective_cache_size = 24GB    # 75% of RAM  
work_mem = 256MB               # Complex aggregations
random_page_cost = 1.1         # SSD optimization

8. Security Hardening

Issues:

• Single database user with DROP TABLE privileges
• No read-only user separation
• No row-level security policies
• No documented credential rotation

Medium Priority (Months 2-3)

9. Table Partitioning for Time-Series Data

• Partition micrograph table by acquisition month
• Partition quality predictions by grid

10. Data Retention & Archival Strategy

Issue: No archival plan for 7-year retention requirement

Capacity Estimate:

• Daily growth: ~10MB metadata
• 7 years: ~25GB metadata, 1-5TB with images
• Requires cold storage strategy

11. Performance Optimization

• Add CHECK constraints for value validation
• Fix timezone handling (use timestamptz)
• Implement partial indexes for common queries
• Add covering indexes to reduce index-only scans

12. Monitoring & Observability

• Enable pg_stat_statements
• Track slow queries
• Monitor table bloat
• Connection pool metrics to Prometheus/Grafana

Database-Specific Concerns

Transaction Management

• No isolation level configured (defaults to READ COMMITTED)
• Risk of phantom reads in multi-step quality predictions
• Recommend: REPEATABLE READ for scientific data consistency

Concurrent Access Patterns

• No row-level locking strategy
• Risk: Multiple agents updating same grid creates race conditions
• Need: SELECT FOR UPDATE in critical sections

Schema Design Issues

Normalization concerns:

• Redundant data: FoilHole stores both gridsquare_uuid (FK) and gridsquare_id (denormalized)
• Duplicate tracking: Both QualityPrediction and CurrentQualityPrediction tables

Type issues:

• Timestamp fields without timezone
• String primary keys without length limits (index bloat)

Implementation Priority Roadmap

Week 1 (Critical Blockers)

• Day 1-2: Implement automated backup system (WAL + pg_dump)
• Day 3: Rollback migration 87f8c5e11906
• Day 4: Create corrected migration (NOT NULL status, CASCADE DELETE)
• Day 5: Re-enable schema drift CI check

Week 2 (High Priority)

• Add unique constraints on natural keys
• Create database test suite (constraints, cascades, migrations)
• Implement connection pool monitoring
• Document disaster recovery procedures

Week 3 (Production Prep)

• Security hardening (user permissions, SSL/TLS)
• PostgreSQL configuration tuning
• Fix index inefficiencies (wrong table bug, consolidation)
• Add CHECK constraints for data validation

Month 2-3 (Operational Excellence)

• Table partitioning for micrographs
• Advanced monitoring (pg_stat_statements, slow queries)
• Archival strategy implementation
• Capacity planning documentation

Assessment Methodology

Two comprehensive reviews conducted:

1. Project Owner Assessment: Strategic overview, 85% rating
2. Database Admin Assessment: Technical deep-dive, 72% rating (more conservative)

Files Analyzed:

• 795 lines in src/smartem_backend/model/database.py
• 5 Alembic migrations
• 42 documentation files
• Infrastructure configurations
• CI/CD workflows

Production Readiness Scorecard

Category	Score	Status
Schema Design	85/100	Good
Migration System	70/100	Fair
Indexes & Performance	90/100	Excellent
Connection Management	85/100	Good
Backup & Recovery	20/100	CRITICAL
Testing Coverage	30/100	Poor
Security	60/100	Fair
Monitoring	40/100	Poor
Documentation	75/100	Good
Disaster Recovery	15/100	CRITICAL

Overall: 57/100 - NOT PRODUCTION READY

References

• Schema drift issue: Schema drift detected: migrations don't match current SQLModel definitions #28 (currently bypassed with continue-on-error: true)
• Duplicate detection tool: tools/find_foilhole_duplicates.py
• Migration file: src/smartem_backend/migrations/versions/2025_10_30_0013-87f8c5e11906_fix_schema_drift_sync_indexes_.py
• Database models: src/smartem_backend/model/database.py

Next Steps

1. Review and prioritize findings with team
2. Allocate 2-3 weeks for critical blocker resolution
3. Implement backup system (Priority 1)
4. Fix schema drift and migration regression (Priority 2)
5. Schedule follow-up assessment after fixes

---

Assessment Date: 2025-11-11
Generated by: Claude Code agents (project-owner + database-admin)

[vredchenko]

Code Snippets for Critical Fixes

1. Backup Script Implementation

Create tools/backup_database.sh:

#!/bin/bash
set -euo pipefail

# Configuration
BACKUP_DIR="/mnt/backup/smartem_db"
RETENTION_DAYS=30
DB_NAME="${POSTGRES_DB:-smartem_db}"
TIMESTAMP=$(date +%Y%m%d_%H%M%S)

# Create backup directory
mkdir -p "${BACKUP_DIR}/daily"

# Full backup with compression
pg_dump -Fc -Z9 "${DB_NAME}" > "${BACKUP_DIR}/daily/smartem_${TIMESTAMP}.dump"

# Clean old backups
find "${BACKUP_DIR}/daily" -name "smartem_*.dump" -mtime +${RETENTION_DAYS} -delete

# Verify backup
pg_restore --list "${BACKUP_DIR}/daily/smartem_${TIMESTAMP}.dump" > /dev/null

echo "Backup completed: smartem_${TIMESTAMP}.dump"

Add to crontab: 0 2 * * * /path/to/tools/backup_database.sh

2. Fix Status Fields Migration

Create new migration to fix nullable status regression:

"""fix status fields to be NOT NULL with proper defaults

Revision ID: <autogenerated>
Revises: 87f8c5e11906
"""

import sqlalchemy as sa
from alembic import op
from sqlalchemy.dialects import postgresql

def upgrade() -> None:
    # Set default values for any NULL status fields first
    op.execute("""
        UPDATE acquisition SET status = 'PLANNED' WHERE status IS NULL;
        UPDATE grid SET status = 'NONE' WHERE status IS NULL;
        UPDATE gridsquare SET status = 'NONE' WHERE status IS NULL;
        UPDATE foilhole SET status = 'NONE' WHERE status IS NULL;
        UPDATE micrograph SET status = 'NONE' WHERE status IS NULL;
    """)
    
    # Make status fields NOT NULL
    op.alter_column('acquisition', 'status',
                   existing_type=postgresql.ENUM('PLANNED', 'STARTED', 'COMPLETED', 
                                                 'PAUSED', 'ABANDONED', name='acquisitionstatus'),
                   nullable=False,
                   existing_server_default=sa.text("'PLANNED'::acquisitionstatus"))
    
    op.alter_column('grid', 'status',
                   existing_type=postgresql.ENUM('NONE', 'SCAN_STARTED', 'SCAN_COMPLETED',
                                                 'GRID_SQUARES_DECISION_STARTED', 
                                                 'GRID_SQUARES_DECISION_COMPLETED', name='gridstatus'),
                   nullable=False,
                   existing_server_default=sa.text("'NONE'::gridstatus"))
    
    # Similar for gridsquare, foilhole, micrograph...

def downgrade() -> None:
    # Revert to nullable (NOT RECOMMENDED)
    op.alter_column('acquisition', 'status', nullable=True)
    # ...

3. Add Unique Constraints Migration

"""add unique constraints on natural keys to prevent duplicates

Revision ID: <autogenerated>
"""

from alembic import op

def upgrade() -> None:
    # Deduplicate existing data first
    op.execute("""
        DELETE FROM foilhole f1
        USING foilhole f2
        WHERE f1.uuid < f2.uuid
          AND f1.foilhole_id = f2.foilhole_id
          AND f1.gridsquare_uuid = f2.gridsquare_uuid
          AND f1.foilhole_id != ''
          AND f1.gridsquare_uuid IS NOT NULL;
    """)
    
    # Add unique constraints
    op.create_unique_constraint(
        'uq_foilhole_natural_key',
        'foilhole',
        ['foilhole_id', 'gridsquare_uuid'],
        postgresql_where="foilhole_id != '' AND gridsquare_uuid IS NOT NULL"
    )
    
    op.create_unique_constraint(
        'uq_gridsquare_natural_key',
        'gridsquare',
        ['gridsquare_id', 'grid_uuid'],
        postgresql_where="gridsquare_id != '' AND grid_uuid IS NOT NULL"
    )
    
    op.create_unique_constraint(
        'uq_micrograph_natural_key',
        'micrograph',
        ['micrograph_id', 'foilhole_uuid'],
        postgresql_where="micrograph_id != '' AND foilhole_uuid IS NOT NULL"
    )

def downgrade() -> None:
    op.drop_constraint('uq_foilhole_natural_key', 'foilhole', type_='unique')
    op.drop_constraint('uq_gridsquare_natural_key', 'gridsquare', type_='unique')
    op.drop_constraint('uq_micrograph_natural_key', 'micrograph', type_='unique')

4. Connection Pool Monitoring

Add to src/smartem_backend/utils.py:

from sqlalchemy import event
import logging

logger = logging.getLogger(__name__)

def setup_postgres_connection(echo=False, force_new=False) -> Engine:
    # ... existing code ...
    
    _db_engine = create_engine(
        connection_string,
        # ... existing params ...
        pool_reset_on_return='rollback',  # NEW: Ensure clean connections
    )
    
    # Add connection pool monitoring
    @event.listens_for(_db_engine, "checkout")
    def receive_checkout(dbapi_conn, connection_record, connection_proxy):
        pool = _db_engine.pool
        logger.debug(f"Connection checked out. Pool size: {pool.size()}, "
                    f"Checked out: {pool.checkedout()}, Overflow: {pool.overflow()}")
        
        # Alert if pool is getting exhausted
        if pool.overflow() > max_overflow * 0.8:
            logger.warning(f"Connection pool 80% exhausted. "
                          f"Consider increasing pool size or investigating leaks.")
    
    return _db_engine

5. Database Test Suite

Create tests/smartem_backend/test_database_constraints.py:

import pytest
from sqlmodel import Session
from smartem_backend.model.database import FoilHole, GridSquare, Grid, Acquisition
from sqlalchemy.exc import IntegrityError

def test_foilhole_natural_key_uniqueness(db_session: Session):
    """Test that duplicate foilhole_id + gridsquare_uuid is prevented"""
    acquisition = Acquisition(uuid="acq-1", name="Test Acquisition")
    grid = Grid(uuid="grid-1", acquisition_uuid="acq-1", name="Test Grid")
    gridsquare = GridSquare(uuid="gs-1", grid_uuid="grid-1", gridsquare_id="GS_001")
    
    db_session.add_all([acquisition, grid, gridsquare])
    db_session.commit()
    
    # Create first foilhole
    foilhole1 = FoilHole(uuid="fh-1", foilhole_id="FH_001", gridsquare_uuid="gs-1")
    db_session.add(foilhole1)
    db_session.commit()
    
    # Attempt duplicate - should raise IntegrityError
    foilhole2 = FoilHole(uuid="fh-2", foilhole_id="FH_001", gridsquare_uuid="gs-1")
    db_session.add(foilhole2)
    
    with pytest.raises(IntegrityError, match="uq_foilhole_natural_key"):
        db_session.commit()

def test_status_field_not_null(db_session: Session):
    """Test that status fields cannot be NULL"""
    acquisition = Acquisition(uuid="acq-1", name="Test", status=None)
    db_session.add(acquisition)
    
    with pytest.raises(IntegrityError, match="null value"):
        db_session.commit()

def test_cascade_delete(db_session: Session):
    """Test that deleting parent cascades to children"""
    acquisition = Acquisition(uuid="acq-1", name="Test")
    grid = Grid(uuid="grid-1", acquisition_uuid="acq-1", name="Grid")
    gridsquare = GridSquare(uuid="gs-1", grid_uuid="grid-1", gridsquare_id="GS_001")
    
    db_session.add_all([acquisition, grid, gridsquare])
    db_session.commit()
    
    # Delete acquisition
    db_session.delete(acquisition)
    db_session.commit()
    
    # Verify cascade worked
    assert db_session.get(Grid, "grid-1") is None
    assert db_session.get(GridSquare, "gs-1") is None

6. Re-enable Schema Drift Check

Edit .github/workflows/_schema_drift.yml:

schema-drift:
  runs-on: ubuntu-latest
  # TODO: Remove continue-on-error after fixing issue DiamondLightSource/smartem-devtools#82
  continue-on-error: false  # Change from true to false