From 1b6a15b4856cc7ef77800078fb2c256ab049ff58 Mon Sep 17 00:00:00 2001
From: wucm667 <stevenwucongmin@gmail.com>
Date: Sat, 23 May 2026 15:04:24 +0800
Subject: [PATCH] fix(db-pool): enforce connection lifetime floors

---
 backend/internal/repository/db_pool.go      | 44 ++++++++++++--
 backend/internal/repository/db_pool_test.go | 65 +++++++++++++++++----
 2 files changed, 93 insertions(+), 16 deletions(-)

diff --git a/backend/internal/repository/db_pool.go b/backend/internal/repository/db_pool.go
index d7116ab1..e110068c 100644
--- a/backend/internal/repository/db_pool.go
+++ b/backend/internal/repository/db_pool.go
@@ -1,12 +1,26 @@
+// Package repository contains persistence infrastructure helpers.
+//
+// DB pool lifetimes are clamped here because lib/pq starts watchCancel
+// goroutines for context-aware queries. If a cloud proxy silently drops idle
+// TCP without RST/FIN, those goroutines can block in Read until database/sql
+// retires the connection. This is a short-term mitigation; the long-term
+// follow-up is migrating PostgreSQL access to jackc/pgx/v5/stdlib.
 package repository
 
 import (
 	"database/sql"
+	"log/slog"
 	"time"
 
 	"github.com/Wei-Shaw/sub2api/internal/config"
 )
 
+const (
+	defaultConnMaxLifetime = 30 * time.Minute
+	defaultConnMaxIdleTime = 5 * time.Minute
+	maxConfiguredConnAge   = 24 * time.Hour
+)
+
 type dbPoolSettings struct {
 	MaxOpenConns    int
 	MaxIdleConns    int
@@ -14,19 +28,41 @@ type dbPoolSettings struct {
 	ConnMaxIdleTime time.Duration
 }
 
-func buildDBPoolSettings(cfg *config.Config) dbPoolSettings {
+func clampDBPoolSettings(cfg *config.Config) dbPoolSettings {
 	return dbPoolSettings{
 		MaxOpenConns:    cfg.Database.MaxOpenConns,
 		MaxIdleConns:    cfg.Database.MaxIdleConns,
-		ConnMaxLifetime: time.Duration(cfg.Database.ConnMaxLifetimeMinutes) * time.Minute,
-		ConnMaxIdleTime: time.Duration(cfg.Database.ConnMaxIdleTimeMinutes) * time.Minute,
+		ConnMaxLifetime: clampDBPoolDuration("database.conn_max_lifetime_minutes", cfg.Database.ConnMaxLifetimeMinutes, defaultConnMaxLifetime),
+		ConnMaxIdleTime: clampDBPoolDuration("database.conn_max_idle_time_minutes", cfg.Database.ConnMaxIdleTimeMinutes, defaultConnMaxIdleTime),
 	}
 }
 
+func clampDBPoolDuration(key string, minutes int, fallback time.Duration) time.Duration {
+	if minutes <= 0 || minutes > int(maxConfiguredConnAge/time.Minute) {
+		slog.Warn("database connection pool duration clamped",
+			"key", key,
+			"before", minutes,
+			"after", int(fallback/time.Minute),
+		)
+		return fallback
+	}
+
+	return time.Duration(minutes) * time.Minute
+}
+
 func applyDBPoolSettings(db *sql.DB, cfg *config.Config) {
-	settings := buildDBPoolSettings(cfg)
+	settings := clampDBPoolSettings(cfg)
 	db.SetMaxOpenConns(settings.MaxOpenConns)
 	db.SetMaxIdleConns(settings.MaxIdleConns)
 	db.SetConnMaxLifetime(settings.ConnMaxLifetime)
 	db.SetConnMaxIdleTime(settings.ConnMaxIdleTime)
+
+	slog.Info("database connection pool configured",
+		slog.Group("effective",
+			slog.Int("max_open", settings.MaxOpenConns),
+			slog.Int("max_idle", settings.MaxIdleConns),
+			slog.Duration("max_lifetime", settings.ConnMaxLifetime),
+			slog.Duration("max_idle_time", settings.ConnMaxIdleTime),
+		),
+	)
 }
diff --git a/backend/internal/repository/db_pool_test.go b/backend/internal/repository/db_pool_test.go
index 3868106a..2757f97c 100644
--- a/backend/internal/repository/db_pool_test.go
+++ b/backend/internal/repository/db_pool_test.go
@@ -11,21 +11,62 @@ import (
 	_ "github.com/lib/pq"
 )
 
-func TestBuildDBPoolSettings(t *testing.T) {
-	cfg := &config.Config{
-		Database: config.DatabaseConfig{
-			MaxOpenConns:           50,
-			MaxIdleConns:           10,
-			ConnMaxLifetimeMinutes: 30,
-			ConnMaxIdleTimeMinutes: 5,
+func TestClampDBPoolSettings(t *testing.T) {
+	tests := []struct {
+		name                string
+		connMaxLifetime     int
+		connMaxIdleTime     int
+		wantMaxLifetime     time.Duration
+		wantConnMaxIdleTime time.Duration
+	}{
+		{
+			name:                "zero values fall back to safe defaults",
+			connMaxLifetime:     0,
+			connMaxIdleTime:     0,
+			wantMaxLifetime:     30 * time.Minute,
+			wantConnMaxIdleTime: 5 * time.Minute,
+		},
+		{
+			name:                "negative values fall back to safe defaults",
+			connMaxLifetime:     -1,
+			connMaxIdleTime:     -5,
+			wantMaxLifetime:     30 * time.Minute,
+			wantConnMaxIdleTime: 5 * time.Minute,
+		},
+		{
+			name:                "reasonable values pass through",
+			connMaxLifetime:     15,
+			connMaxIdleTime:     3,
+			wantMaxLifetime:     15 * time.Minute,
+			wantConnMaxIdleTime: 3 * time.Minute,
+		},
+		{
+			name:                "values over twenty four hours fall back to safe defaults",
+			connMaxLifetime:     24*60 + 1,
+			connMaxIdleTime:     24*60 + 1,
+			wantMaxLifetime:     30 * time.Minute,
+			wantConnMaxIdleTime: 5 * time.Minute,
 		},
 	}
 
-	settings := buildDBPoolSettings(cfg)
-	require.Equal(t, 50, settings.MaxOpenConns)
-	require.Equal(t, 10, settings.MaxIdleConns)
-	require.Equal(t, 30*time.Minute, settings.ConnMaxLifetime)
-	require.Equal(t, 5*time.Minute, settings.ConnMaxIdleTime)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := &config.Config{
+				Database: config.DatabaseConfig{
+					MaxOpenConns:           50,
+					MaxIdleConns:           10,
+					ConnMaxLifetimeMinutes: tt.connMaxLifetime,
+					ConnMaxIdleTimeMinutes: tt.connMaxIdleTime,
+				},
+			}
+
+			settings := clampDBPoolSettings(cfg)
+			require.Equal(t, 50, settings.MaxOpenConns)
+			require.Equal(t, 10, settings.MaxIdleConns)
+			require.Equal(t, tt.wantMaxLifetime, settings.ConnMaxLifetime)
+			require.Equal(t, tt.wantConnMaxIdleTime, settings.ConnMaxIdleTime)
+		})
+	}
 }
 
 func TestApplyDBPoolSettings(t *testing.T) {