Skip to content

Commit 20d1b44

Browse files
EDsCODEclaude
andcommitted
Fix connection hanging with pooled in-memory database
- Pool sql.DB connections per user to avoid DuckDB file locking issues from rapid open/close cycles - Use in-memory database (:memory:) when DuckLake is configured since data lives in RDS/S3 anyway - eliminates file locking entirely - Share connection across all clients for the same user - Add debug logging for connection lifecycle (can be removed later) The root cause was DuckDB file locking: when multiple clients rapidly opened and closed the same database file, DuckDB wouldn't fully release the file lock before the next connection tried to open it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 87542b6 commit 20d1b44

File tree

2 files changed

+64
-22
lines changed

2 files changed

+64
-22
lines changed

server/conn.go

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,22 +77,15 @@ func (c *clientConn) serve() error {
7777
return fmt.Errorf("startup failed: %w", err)
7878
}
7979

80-
// Create a new DuckDB connection for this client session.
81-
// Each client gets its own connection to ensure proper isolation of
82-
// temporary tables and session state, matching PostgreSQL's behavior.
83-
db, err := c.server.createDBConnection(c.username)
80+
// Get a DuckDB connection from the pool. Connections are shared across
81+
// clients to avoid DuckDB file locking issues from rapid open/close cycles.
82+
db, err := c.server.getDBConnection(c.username)
8483
if err != nil {
8584
c.sendError("FATAL", "28000", fmt.Sprintf("failed to open database: %v", err))
8685
return err
8786
}
8887
c.db = db
89-
// Ensure the database connection is closed when this client disconnects
90-
defer func() {
91-
if c.db != nil {
92-
c.db.Close()
93-
log.Printf("Closed DuckDB connection for user %q", c.username)
94-
}
95-
}()
88+
// Note: Don't close the connection - it's pooled and shared across clients
9689

9790
// Send initial parameters
9891
c.sendInitialParams()

server/server.go

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ type Server struct {
9494
// duckLakeSem serializes DuckLake attachment to avoid write-write conflicts.
9595
// Using a channel instead of mutex allows for timeout on acquisition.
9696
duckLakeSem chan struct{}
97+
98+
// dbPool caches sql.DB connections per database file to avoid DuckDB file locking
99+
// issues from rapid open/close cycles. Connections are created on first use and
100+
// kept open for the server lifetime.
101+
dbPool map[string]*sql.DB
102+
dbPoolMu sync.Mutex
97103
}
98104

99105
func New(cfg Config) (*Server, error) {
@@ -129,6 +135,7 @@ func New(cfg Config) (*Server, error) {
129135
Certificates: []tls.Certificate{cert},
130136
},
131137
duckLakeSem: make(chan struct{}, 1),
138+
dbPool: make(map[string]*sql.DB),
132139
}
133140

134141
log.Printf("TLS enabled with certificate: %s", cfg.TLSCertFile)
@@ -248,33 +255,65 @@ func (s *Server) ActiveConnections() int64 {
248255
return atomic.LoadInt64(&s.activeConns)
249256
}
250257

251-
// createDBConnection creates a new DuckDB connection for a client session.
252-
// Each client connection gets its own DB connection to ensure proper isolation
253-
// of temporary tables and session state, matching PostgreSQL's behavior.
254-
func (s *Server) createDBConnection(username string) (*sql.DB, error) {
255-
dbPath := fmt.Sprintf("%s/%s.db", s.cfg.DataDir, username)
258+
// getDBConnection returns a DuckDB connection for a client session.
259+
// Connections are pooled per database file to avoid DuckDB file locking issues
260+
// from rapid open/close cycles. The connection is shared across all clients
261+
// using the same database file.
262+
func (s *Server) getDBConnection(username string) (*sql.DB, error) {
263+
// Use in-memory database when DuckLake is configured (data lives in RDS/S3),
264+
// otherwise use file-based database for local storage.
265+
var dbPath string
266+
if s.cfg.DuckLake.MetadataStore != "" {
267+
dbPath = ":memory:"
268+
} else {
269+
dbPath = fmt.Sprintf("%s/%s.db", s.cfg.DataDir, username)
270+
}
271+
272+
// Check if we already have a connection for this database
273+
poolKey := fmt.Sprintf("%s:%s", username, dbPath)
274+
s.dbPoolMu.Lock()
275+
if db, ok := s.dbPool[poolKey]; ok {
276+
s.dbPoolMu.Unlock()
277+
// Verify connection is still alive
278+
if err := db.Ping(); err == nil {
279+
log.Printf("[%s] Reusing pooled DuckDB connection", username)
280+
return db, nil
281+
}
282+
// Connection is dead, remove from pool and create new one
283+
log.Printf("[%s] Pooled connection dead, creating new one", username)
284+
s.dbPoolMu.Lock()
285+
delete(s.dbPool, poolKey)
286+
}
287+
s.dbPoolMu.Unlock()
288+
289+
// Create new connection
290+
log.Printf("[%s] Opening DuckDB at %s", username, dbPath)
256291
db, err := sql.Open("duckdb", dbPath)
257292
if err != nil {
258293
return nil, fmt.Errorf("failed to open duckdb: %w", err)
259294
}
260295

261-
// Limit the connection pool to a single connection per client session.
262-
// This prevents resource exhaustion from too many DuckDB connections.
263-
db.SetMaxOpenConns(1)
264-
db.SetMaxIdleConns(1)
296+
// Configure connection pool - allow multiple concurrent queries since
297+
// this connection is shared across all clients for this database
298+
db.SetMaxOpenConns(10)
299+
db.SetMaxIdleConns(5)
265300
db.SetConnMaxLifetime(30 * time.Minute)
266301

267302
// Verify connection
303+
log.Printf("[%s] Pinging DuckDB...", username)
268304
if err := db.Ping(); err != nil {
269305
db.Close()
270306
return nil, fmt.Errorf("failed to ping duckdb: %w", err)
271307
}
308+
log.Printf("[%s] Ping successful", username)
272309

273310
// Load configured extensions
311+
log.Printf("[%s] Loading extensions...", username)
274312
if err := s.loadExtensions(db); err != nil {
275313
log.Printf("Warning: failed to load some extensions for user %q: %v", username, err)
276314
// Continue anyway - database will still work without the extensions
277315
}
316+
log.Printf("[%s] Extensions loaded", username)
278317

279318
// Attach DuckLake catalog if configured
280319
if err := s.attachDuckLake(db); err != nil {
@@ -294,7 +333,12 @@ func (s *Server) createDBConnection(username string) (*sql.DB, error) {
294333
// Continue anyway - basic queries will still work
295334
}
296335

297-
log.Printf("Opened DuckDB connection for user %q at %s", username, dbPath)
336+
// Add to pool
337+
s.dbPoolMu.Lock()
338+
s.dbPool[poolKey] = db
339+
s.dbPoolMu.Unlock()
340+
341+
log.Printf("[%s] Created new pooled DuckDB connection at %s", username, dbPath)
298342
return db, nil
299343
}
300344

@@ -337,9 +381,14 @@ func (s *Server) attachDuckLake(db *sql.DB) error {
337381
// "database with name '__ducklake_metadata_ducklake' already exists".
338382
// Use a 30-second timeout to prevent connections from hanging indefinitely
339383
// if attachment is slow (e.g., network latency to metadata store).
384+
log.Printf("Waiting for DuckLake attachment lock...")
340385
select {
341386
case s.duckLakeSem <- struct{}{}:
342-
defer func() { <-s.duckLakeSem }()
387+
log.Printf("Acquired DuckLake attachment lock")
388+
defer func() {
389+
<-s.duckLakeSem
390+
log.Printf("Released DuckLake attachment lock")
391+
}()
343392
case <-time.After(30 * time.Second):
344393
return fmt.Errorf("timeout waiting for DuckLake attachment lock")
345394
}

0 commit comments

Comments
 (0)