mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
chore: close db properly in early exit paths in ConnectToPostgres (#18448)
There were some code paths where if we exited early from the function the postgres connection would never get cleaned up. This is the mechanism that cleans up the db - it requires the err variable to be not nil: https://github.com/coder/coder/blob/118bf981454188c4989e8b565dec67906616f885/cli/server.go#L2319-L2328
This commit is contained in:
+5
-8
@@ -2312,19 +2312,20 @@ func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, d
|
||||
|
||||
var err error
|
||||
var sqlDB *sql.DB
|
||||
dbNeedsClosing := true
|
||||
// Try to connect for 30 seconds.
|
||||
ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
|
||||
defer cancel()
|
||||
|
||||
defer func() {
|
||||
if err == nil {
|
||||
if !dbNeedsClosing {
|
||||
return
|
||||
}
|
||||
if sqlDB != nil {
|
||||
_ = sqlDB.Close()
|
||||
sqlDB = nil
|
||||
logger.Debug(ctx, "closed db before returning from ConnectToPostgres")
|
||||
}
|
||||
logger.Error(ctx, "connect to postgres failed", slog.Error(err))
|
||||
}()
|
||||
|
||||
var tries int
|
||||
@@ -2361,12 +2362,7 @@ func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, d
|
||||
}
|
||||
defer version.Close()
|
||||
if !version.Next() {
|
||||
// it's critical we assign to the err variable, otherwise the defer statement
|
||||
// that runs db.Close() will not execute it
|
||||
if err = version.Err(); err != nil {
|
||||
return nil, xerrors.Errorf("no rows returned for version select: %w", err)
|
||||
}
|
||||
return nil, xerrors.Errorf("no rows returned for version select")
|
||||
return nil, xerrors.Errorf("no rows returned for version select: %w", version.Err())
|
||||
}
|
||||
var versionNum int
|
||||
err = version.Scan(&versionNum)
|
||||
@@ -2408,6 +2404,7 @@ func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, d
|
||||
// of connection churn.
|
||||
sqlDB.SetMaxIdleConns(3)
|
||||
|
||||
dbNeedsClosing = false
|
||||
return sqlDB, nil
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user