diff --git a/cmd/qfs/main.go b/cmd/qfs/main.go index 22d2109..d7d898c 100644 --- a/cmd/qfs/main.go +++ b/cmd/qfs/main.go @@ -168,6 +168,13 @@ func main() { // Create connection manager. Runtime stays local-first; MariaDB is used on demand by sync/setup only. connMgr := db.NewConnectionManager(local) + // Keep the online-status cache fresh in the background so request-handling + // goroutines (health checks, middleware, etc.) never block on the MySQL + // dial/read timeout themselves. + connMgrCtx, connMgrCancel := context.WithCancel(context.Background()) + defer connMgrCancel() + connMgr.Start(connMgrCtx) + dbUser := local.GetDBUser() slog.Info("starting QuoteForge server", @@ -272,6 +279,8 @@ func main() { syncWorker.Stop() workerCancel() backupCancel() + connMgr.Stop() + connMgrCancel() // Then shutdown HTTP server ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) diff --git a/internal/db/connection.go b/internal/db/connection.go index a3cc2c1..aaa9b95 100644 --- a/internal/db/connection.go +++ b/internal/db/connection.go @@ -17,6 +17,11 @@ const ( defaultPingInterval = 30 * time.Second defaultReconnectCooldown = 10 * time.Second + // defaultStatusCheckInterval controls how often the background prober + // re-checks connectivity to keep IsOnline() cheap. Request-handling + // goroutines must never pay the MySQL dial/read timeout themselves. + defaultStatusCheckInterval = 15 * time.Second + maxOpenConns = 10 maxIdleConns = 2 connMaxLifetime = 5 * time.Minute @@ -33,86 +38,148 @@ type ConnectionStatus struct { // ConnectionManager manages database connections with thread-safety and connection pooling type ConnectionManager struct { localDB *localdb.LocalDB // for getting DSN from settings - mu sync.RWMutex // protects db and state + mu sync.RWMutex // protects db/lastError/lastCheck only — never held during network I/O + connMu sync.Mutex // serializes actual dial/ping attempts; held *instead of* mu during network I/O db *gorm.DB // current connection (nil if not connected) lastError error // last connection error lastCheck time.Time // time of last check/attempt connectTimeout time.Duration // timeout for connection (default: 5s) pingInterval time.Duration // minimum interval between pings (default: 30s) reconnectCooldown time.Duration // pause after failed attempt (default: 10s) + + statusCheckInterval time.Duration // background prober cadence (default: 15s) + stopStatusLoop chan struct{} // closed by Stop() to end the background loop } // NewConnectionManager creates a new ConnectionManager instance func NewConnectionManager(localDB *localdb.LocalDB) *ConnectionManager { return &ConnectionManager{ - localDB: localDB, - connectTimeout: defaultConnectTimeout, - pingInterval: defaultPingInterval, - reconnectCooldown: defaultReconnectCooldown, - db: nil, - lastError: nil, - lastCheck: time.Time{}, + localDB: localDB, + connectTimeout: defaultConnectTimeout, + pingInterval: defaultPingInterval, + reconnectCooldown: defaultReconnectCooldown, + statusCheckInterval: defaultStatusCheckInterval, + db: nil, + lastError: nil, + lastCheck: time.Time{}, } } -// GetDB returns the current database connection, establishing it if needed -// Thread-safe and respects connection cooldowns +// Start launches a background goroutine that keeps the online-status cache +// fresh, so that IsOnline() (called from request-handling middleware) never +// blocks on network I/O itself. Returns immediately — the app must be able +// to serve the local-first UI right away, before connectivity is even known. +// Until the first background check completes, IsOnline() reports offline +// (the safe default). Stop via ctx cancellation or Stop(). +func (cm *ConnectionManager) Start(ctx context.Context) { + cm.mu.Lock() + if cm.stopStatusLoop == nil { + cm.stopStatusLoop = make(chan struct{}) + } + stopCh := cm.stopStatusLoop + cm.mu.Unlock() + + go func() { + // Prime the cache in the background; the dial/read timeout must not + // delay server startup. + cm.checkOnlineNow() + + ticker := time.NewTicker(cm.statusCheckInterval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return + case <-stopCh: + return + case <-ticker.C: + cm.checkOnlineNow() + } + } + }() +} + +// Stop ends the background status-refresh loop started by Start. +func (cm *ConnectionManager) Stop() { + cm.mu.Lock() + defer cm.mu.Unlock() + if cm.stopStatusLoop != nil { + close(cm.stopStatusLoop) + cm.stopStatusLoop = nil + } +} + +// GetDB returns the current database connection, establishing it if needed. +// Thread-safe and respects connection cooldowns. The actual network I/O +// (dial/ping) never runs while holding cm.mu, so concurrent readers of the +// cached status (IsOnline, GetStatus) are never blocked by an in-flight +// connection attempt — only concurrent GetDB/checkOnlineNow callers are +// serialized against each other, via connMu. func (cm *ConnectionManager) GetDB() (*gorm.DB, error) { // Handle case where localDB is nil if cm.localDB == nil { return nil, fmt.Errorf("local database not initialized") } - // First check if we already have a valid connection - cm.mu.RLock() - if cm.db != nil { - // Check if connection is still valid and within ping interval - if time.Since(cm.lastCheck) < cm.pingInterval { - cm.mu.RUnlock() - return cm.db, nil - } + if db, err, ok := cm.cachedResult(); ok { + return db, err } - cm.mu.RUnlock() - // Upgrade to write lock + // Serialize actual connection attempts so concurrent callers don't dial + // in parallel. This may block the calling goroutine on network I/O, but + // never blocks other goroutines that only read the cached status. + cm.connMu.Lock() + defer cm.connMu.Unlock() + + // Re-check: another goroutine may have just finished connecting while we + // were waiting for connMu. + if db, err, ok := cm.cachedResult(); ok { + return db, err + } + + newDB, err := cm.dial() + cm.mu.Lock() - defer cm.mu.Unlock() - - // Double-check: someone else might have connected while we were waiting for the write lock - if cm.db != nil { - // Check if connection is still valid and within ping interval - if time.Since(cm.lastCheck) < cm.pingInterval { - return cm.db, nil - } - } - - // Check if we're in cooldown period after a failed attempt - if cm.lastError != nil && time.Since(cm.lastCheck) < cm.reconnectCooldown { - return nil, cm.lastError - } - - // Attempt to connect - err := cm.connect() if err != nil { // Drop stale handle so callers don't treat it as an active connection. cm.db = nil cm.lastError = err cm.lastCheck = time.Now() + cm.mu.Unlock() return nil, err } - - // Update last check time and return success - cm.lastCheck = time.Now() + cm.db = newDB cm.lastError = nil - return cm.db, nil + cm.lastCheck = time.Now() + cm.mu.Unlock() + + return newDB, nil } -// connect establishes a new database connection -func (cm *ConnectionManager) connect() error { +// cachedResult returns (db, err, true) if the cached state is still fresh +// enough to answer without a new network round-trip: either a live +// connection within pingInterval, or a recent failure still within +// reconnectCooldown. Returns ok=false if a fresh connection attempt is needed. +func (cm *ConnectionManager) cachedResult() (*gorm.DB, error, bool) { + cm.mu.RLock() + defer cm.mu.RUnlock() + + if cm.db != nil && time.Since(cm.lastCheck) < cm.pingInterval { + return cm.db, nil, true + } + if cm.db == nil && cm.lastError != nil && time.Since(cm.lastCheck) < cm.reconnectCooldown { + return nil, cm.lastError, true + } + return nil, nil, false +} + +// dial establishes a new database connection. Pure network I/O — must not be +// called while holding cm.mu. +func (cm *ConnectionManager) dial() (*gorm.DB, error) { // Get DSN from local settings dsn, err := cm.localDB.GetDSN() if err != nil { - return fmt.Errorf("getting DSN: %w", err) + return nil, fmt.Errorf("getting DSN: %w", err) } // Create context with timeout @@ -124,18 +191,18 @@ func (cm *ConnectionManager) connect() error { Logger: logger.Default.LogMode(logger.Silent), }) if err != nil { - return fmt.Errorf("opening database connection: %w", err) + return nil, fmt.Errorf("opening database connection: %w", err) } // Test the connection sqlDB, err := db.DB() if err != nil { - return fmt.Errorf("getting sql.DB: %w", err) + return nil, fmt.Errorf("getting sql.DB: %w", err) } // Ping with timeout if err = sqlDB.PingContext(ctx); err != nil { - return fmt.Errorf("pinging database: %w", err) + return nil, fmt.Errorf("pinging database: %w", err) } // Set connection pool settings @@ -143,15 +210,25 @@ func (cm *ConnectionManager) connect() error { sqlDB.SetMaxIdleConns(maxIdleConns) sqlDB.SetConnMaxLifetime(connMaxLifetime) - // Store the connection - cm.db = db - - return nil + return db, nil } -// IsOnline checks if the database is currently connected and responsive. -// If disconnected, it tries to reconnect (respecting cooldowns in GetDB). +// IsOnline returns the cached connectivity status. It never performs network +// I/O itself so it is safe to call from request-handling middleware; the +// background loop started by Start() (or an explicit TryConnect) is +// responsible for keeping the cache fresh. func (cm *ConnectionManager) IsOnline() bool { + cm.mu.RLock() + defer cm.mu.RUnlock() + return cm.db != nil && cm.lastError == nil +} + +// checkOnlineNow checks if the database is currently connected and +// responsive, performing real network I/O (dial/ping) as needed. If +// disconnected, it tries to reconnect (respecting cooldowns in GetDB). This +// must only be called from the background status loop or explicit +// user-triggered reconnects, never from request-handling goroutines. +func (cm *ConnectionManager) checkOnlineNow() bool { cm.mu.RLock() isDisconnected := cm.db == nil lastErr := cm.lastError @@ -169,57 +246,65 @@ func (cm *ConnectionManager) IsOnline() bool { return lastErr == nil } - // Need to perform actual ping. - cm.mu.Lock() - defer cm.mu.Unlock() + // Serialize actual ping attempts (network I/O) against other + // connect/ping attempts, without ever holding cm.mu during the I/O. + cm.connMu.Lock() + defer cm.connMu.Unlock() - // Double-check after acquiring write lock - if cm.db == nil { + cm.mu.RLock() + db := cm.db + checkedRecently = time.Since(cm.lastCheck) < cm.pingInterval + cm.mu.RUnlock() + + if db == nil { return false } + if checkedRecently { + return true + } - // Perform ping with timeout + // Perform ping with timeout — no locks held here. ctx, cancel := context.WithTimeout(context.Background(), cm.connectTimeout) defer cancel() - sqlDB, err := cm.db.DB() + sqlDB, err := db.DB() + if err == nil { + err = sqlDB.PingContext(ctx) + } + + cm.mu.Lock() + defer cm.mu.Unlock() if err != nil { cm.lastError = err cm.lastCheck = time.Now() cm.db = nil return false } - - if err = sqlDB.PingContext(ctx); err != nil { - cm.lastError = err - cm.lastCheck = time.Now() - cm.db = nil - return false - } - - // Update last check time and return success cm.lastCheck = time.Now() cm.lastError = nil return true } -// TryConnect forces a new connection attempt (for UI "Reconnect" button) -// Ignores cooldown period +// TryConnect forces a new connection attempt (for UI "Reconnect" button). +// Ignores the reconnect cooldown, but still serializes against other +// dial attempts via connMu and never holds cm.mu during network I/O. func (cm *ConnectionManager) TryConnect() error { + cm.connMu.Lock() + defer cm.connMu.Unlock() + + newDB, err := cm.dial() + cm.mu.Lock() defer cm.mu.Unlock() - - // Attempt to connect - err := cm.connect() if err != nil { + cm.db = nil cm.lastError = err cm.lastCheck = time.Now() return err } - - // Update last check time and clear error - cm.lastCheck = time.Now() + cm.db = newDB cm.lastError = nil + cm.lastCheck = time.Now() return nil }