From 2885d8c611c6ace581178fbc1fb667bb4de6e77a Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 11 Nov 2024 14:59:40 -0600 Subject: [PATCH 1/2] manage idle connections in the pgx pool --- app/app.go | 3 --- app/cmd.go | 17 +++++++++++++---- app/pause.go | 7 ------- swo/manager.go | 5 ++++- swo/pgxpool.go | 7 ++++++- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/app/app.go b/app/app.go index 45172fdd32..a5da92fcf0 100644 --- a/app/app.go +++ b/app/app.go @@ -202,9 +202,6 @@ func NewApp(c Config, pool *pgxpool.Pool) (*App, error) { } } - app.db.SetMaxIdleConns(c.DBMaxIdle) - app.db.SetMaxOpenConns(c.DBMaxOpen) - app.mgr = lifecycle.NewManager(app._Run, app._Shutdown) err = app.mgr.SetStartupFunc(app.startup) if err != nil { diff --git a/app/cmd.go b/app/cmd.go index a6b0903b17..1ad9247ced 100644 --- a/app/cmd.go +++ b/app/cmd.go @@ -150,7 +150,14 @@ Available Flags: return errors.Wrap(err, "nextdb") } - mgr, err := swo.NewManager(swo.Config{OldDBURL: cfg.DBURL, NewDBURL: cfg.DBURLNext, CanExec: !cfg.APIOnly, Logger: cfg.LegacyLogger}) + mgr, err := swo.NewManager(swo.Config{ + OldDBURL: cfg.DBURL, + NewDBURL: cfg.DBURLNext, + CanExec: !cfg.APIOnly, + Logger: cfg.LegacyLogger, + MaxOpen: cfg.DBMaxOpen, + MaxIdle: cfg.DBMaxIdle, + }) if err != nil { return errors.Wrap(err, "init switchover handler") } @@ -162,13 +169,15 @@ Available Flags: return errors.Wrap(err, "connect to postgres") } - cfg, err := pgxpool.ParseConfig(appURL) + poolCfg, err := pgxpool.ParseConfig(appURL) if err != nil { return errors.Wrap(err, "parse db URL") } - sqldrv.SetConfigRetries(cfg) + poolCfg.MaxConns = int32(cfg.DBMaxOpen) + poolCfg.MinConns = int32(cfg.DBMaxIdle) + sqldrv.SetConfigRetries(poolCfg) - pool, err = pgxpool.NewWithConfig(context.Background(), cfg) + pool, err = pgxpool.NewWithConfig(context.Background(), poolCfg) if err != nil { return errors.Wrap(err, "connect to postgres") } diff --git a/app/pause.go b/app/pause.go index b9eb4d656b..d0d21efac3 100644 --- a/app/pause.go +++ b/app/pause.go @@ -2,7 +2,6 @@ package app import ( "context" - "time" ) // LogBackgroundContext returns a context.Background with the application logger configured. @@ -19,17 +18,11 @@ func (app *App) Resume(ctx context.Context) error { } func (app *App) _pause(ctx context.Context) error { - app.db.SetMaxIdleConns(0) - app.db.SetConnMaxLifetime(time.Second) - app.db.SetMaxOpenConns(3) app.events.Stop() return nil } func (app *App) _resume(ctx context.Context) error { - app.db.SetMaxOpenConns(app.cfg.DBMaxOpen) - app.db.SetMaxIdleConns(app.cfg.DBMaxIdle) - app.db.SetConnMaxLifetime(0) app.events.Start() return nil diff --git a/swo/manager.go b/swo/manager.go index 9e44b4edc4..c63667a3db 100644 --- a/swo/manager.go +++ b/swo/manager.go @@ -57,6 +57,9 @@ type Config struct { OldDBURL, NewDBURL string CanExec bool Logger *log.Logger + + MaxOpen int + MaxIdle int } // NewManager will create a new Manager with the given configuration. @@ -97,7 +100,7 @@ func NewManager(cfg Config) (*Manager, error) { return nil, fmt.Errorf("connect to new db: %w", err) } - appPgx, err := NewAppPGXPool(appMainURL, appNextURL) + appPgx, err := NewAppPGXPool(appMainURL, appNextURL, cfg.MaxOpen, cfg.MaxIdle) if err != nil { return nil, fmt.Errorf("create pool: %w", err) } diff --git a/swo/pgxpool.go b/swo/pgxpool.go index ace1c23582..023ef0fbd9 100644 --- a/swo/pgxpool.go +++ b/swo/pgxpool.go @@ -16,17 +16,22 @@ import ( // // Until the switchover is complete, the old database will be protected with a // shared advisory lock (4369). -func NewAppPGXPool(oldURL, nextURL string) (*pgxpool.Pool, error) { +func NewAppPGXPool(oldURL, nextURL string, maxOpen, maxIdle int) (*pgxpool.Pool, error) { cfg, err := pgxpool.ParseConfig(oldURL) if err != nil { return nil, fmt.Errorf("parse old URL: %w", err) } sqldrv.SetConfigRetries(cfg) + cfg.MaxConns = int32(maxOpen) + cfg.MinConns = int32(maxIdle) + nextCfg, err := pgxpool.ParseConfig(nextURL) if err != nil { return nil, fmt.Errorf("parse next URL: %w", err) } sqldrv.SetConfigRetries(nextCfg) + nextCfg.MaxConns = int32(maxOpen) + nextCfg.MinConns = int32(maxIdle) var mx sync.Mutex var isDone bool From da0e075873d564724077d236495e8a9c9b98baa1 Mon Sep 17 00:00:00 2001 From: Nathaniel Caza Date: Mon, 11 Nov 2024 15:13:35 -0600 Subject: [PATCH 2/2] migrate: add verification for latest migration consistency --- app/cmd.go | 5 +++++ migrate/migrate.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/app/cmd.go b/app/cmd.go index 1ad9247ced..caf0b2f2d4 100644 --- a/app/cmd.go +++ b/app/cmd.go @@ -145,6 +145,11 @@ Available Flags: var pool *pgxpool.Pool if cfg.DBURLNext != "" { + err = migrate.VerifyIsLatest(ctx, cfg.DBURL) + if err != nil { + return errors.Wrap(err, "verify db") + } + err = doMigrations(cfg.DBURLNext) if err != nil { return errors.Wrap(err, "nextdb") diff --git a/migrate/migrate.go b/migrate/migrate.go index bf0fabe334..ddfe1301a8 100644 --- a/migrate/migrate.go +++ b/migrate/migrate.go @@ -39,6 +39,15 @@ func migrationIDs() []string { return ids } +// LatestID will return the ID of the latest migration. +func LatestID() string { + ids := migrationIDs() + if len(ids) == 0 { + return "" + } + return ids[len(ids)-1] +} + // Names will return all AssetNames without the timestamps and extensions func Names() []string { uniq := make(map[string]struct{}) @@ -74,6 +83,29 @@ func migrationID(name string) (int, string) { return -1, "" } +// VerifyIsLatest will verify the latest migration is the same as the latest available migration. +// +// This ensures the DB isn't newer than the currently running code. +func VerifyIsLatest(ctx context.Context, url string) error { + conn, err := getConn(ctx, url) + if err != nil { + return err + } + defer conn.Close(ctx) + + var dbLatest string + err = conn.QueryRow(ctx, `select id from gorp_migrations order by id desc limit 1`).Scan(&dbLatest) + if err != nil { + return fmt.Errorf("read latest migration from DB: %w", err) + } + + if dbLatest != LatestID() { + return errors.Errorf("latest migration in DB is '%s' but expected '%s'; local GoAlert version is likely older than the DB's latest migration (not allowed in SWO-mode)", dbLatest, LatestID()) + } + + return nil +} + // VerifyAll will verify all migrations have already been applied. func VerifyAll(ctx context.Context, url string) error { ids := migrationIDs()