Skip to content

Commit 87542b6

Browse files
EDsCODEclaude
andcommitted
Replace mutex with timed semaphore for DuckLake attachment
- Revert double-checked locking (ATTACH is session-scoped, doesn't persist) - Replace sync.Mutex with channel-based semaphore - Add 30-second timeout to prevent indefinite blocking - Connections will fail with clear error if attachment takes too long This prevents connection backups when DuckLake attachment is slow (e.g., network latency to RDS metadata store). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent fbe11a4 commit 87542b6

File tree

1 file changed

+18
-22
lines changed

1 file changed

+18
-22
lines changed

server/server.go

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,9 @@ type Server struct {
9191
closeMu sync.Mutex
9292
activeConns int64 // atomic counter for active connections
9393

94-
// duckLakeMu serializes DuckLake secret creation to avoid write-write conflicts
95-
duckLakeMu sync.Mutex
94+
// duckLakeSem serializes DuckLake attachment to avoid write-write conflicts.
95+
// Using a channel instead of mutex allows for timeout on acquisition.
96+
duckLakeSem chan struct{}
9697
}
9798

9899
func New(cfg Config) (*Server, error) {
@@ -127,6 +128,7 @@ func New(cfg Config) (*Server, error) {
127128
tlsConfig: &tls.Config{
128129
Certificates: []tls.Certificate{cert},
129130
},
131+
duckLakeSem: make(chan struct{}, 1),
130132
}
131133

132134
log.Printf("TLS enabled with certificate: %s", cfg.TLSCertFile)
@@ -330,27 +332,21 @@ func (s *Server) attachDuckLake(db *sql.DB) error {
330332
return nil // DuckLake not configured
331333
}
332334

333-
// Fast path: check if DuckLake is already attached (no mutex needed)
334-
// This avoids blocking on the mutex for the common case where DuckLake
335-
// was already attached by an earlier connection.
336-
var count int
337-
err := db.QueryRow("SELECT COUNT(*) FROM duckdb_databases() WHERE database_name = 'ducklake'").Scan(&count)
338-
if err == nil && count > 0 {
339-
// Already attached, just set as default
340-
if _, err := db.Exec("USE ducklake"); err != nil {
341-
return fmt.Errorf("failed to set DuckLake as default catalog: %w", err)
342-
}
343-
return nil
335+
// Serialize DuckLake attachment to avoid race conditions where multiple
336+
// connections try to attach simultaneously, causing errors like
337+
// "database with name '__ducklake_metadata_ducklake' already exists".
338+
// Use a 30-second timeout to prevent connections from hanging indefinitely
339+
// if attachment is slow (e.g., network latency to metadata store).
340+
select {
341+
case s.duckLakeSem <- struct{}{}:
342+
defer func() { <-s.duckLakeSem }()
343+
case <-time.After(30 * time.Second):
344+
return fmt.Errorf("timeout waiting for DuckLake attachment lock")
344345
}
345346

346-
// Slow path: need to attach DuckLake. Serialize to avoid race conditions
347-
// where multiple connections try to attach simultaneously, causing
348-
// "database with name '__ducklake_metadata_ducklake' already exists" errors
349-
s.duckLakeMu.Lock()
350-
defer s.duckLakeMu.Unlock()
351-
352-
// Double-check after acquiring mutex (another goroutine may have attached)
353-
err = db.QueryRow("SELECT COUNT(*) FROM duckdb_databases() WHERE database_name = 'ducklake'").Scan(&count)
347+
// Check if DuckLake catalog is already attached
348+
var count int
349+
err := db.QueryRow("SELECT COUNT(*) FROM duckdb_databases() WHERE database_name = 'ducklake'").Scan(&count)
354350
if err == nil && count > 0 {
355351
// Already attached, just set as default
356352
if _, err := db.Exec("USE ducklake"); err != nil {
@@ -410,7 +406,7 @@ func (s *Server) attachDuckLake(db *sql.DB) error {
410406
// - "config": explicit credentials (for MinIO or when you have access keys)
411407
// - "credential_chain": AWS SDK credential chain (env vars, config files, instance metadata, etc.)
412408
//
413-
// Note: Caller must hold duckLakeMu to avoid race conditions.
409+
// Note: Caller must hold duckLakeSem to avoid race conditions.
414410
// See: https://duckdb.org/docs/stable/core_extensions/httpfs/s3api
415411
func (s *Server) createS3Secret(db *sql.DB) error {
416412
// Check if secret already exists to avoid unnecessary creation

0 commit comments

Comments
 (0)