Skip to content

Commit

Permalink
testdb.Manager: try to recreate pool after cleanup error
Browse files Browse the repository at this point in the history
In recent changes from #769 / fb6b3e9, I inadvertently tweaked the logic
of the testdb.Manager to throw away / destroy a resource if it
encountered cleanup errors. Destroying the resource has the side effect
of causing the manager to try to reallocate a new database with a new
number, potentially higher than the number of test databases we've
created.

Instead, try to just make a new pool on the same database, and only
destroy the resource if that fails.
  • Loading branch information
bgentry committed Feb 19, 2025
1 parent f0eaa64 commit bacf8b5
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
25 changes: 21 additions & 4 deletions internal/testdb/db_with_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,10 @@ func (db *DBWithPool) release() {
if errors.Is(err, puddle.ErrClosedPool) {
db.logger.Debug("DBWithPool: pool is closed, re-creating", "dbName", db.dbName)

newPgxPool, err := pgxpool.NewWithConfig(ctx, db.res.Value().config)
if err != nil {
if err := db.recreatePool(ctx); err != nil {
db.res.Destroy()
return
}
db.res.Value().pool = newPgxPool
} else {
// Log any other ping error but proceed with cleanup.
db.logger.Debug("DBWithPool: pool ping returned error", "dbName", db.dbName, "err", err)
Expand All @@ -56,7 +54,11 @@ func (db *DBWithPool) release() {
db.logger.Debug("DBWithPool: release calling cleanup", "dbName", db.dbName)
if err := db.manager.cleanup(ctx, db.res.Value().pool); err != nil {
db.logger.Error("testdb.DBWithPool: Error during release cleanup", "err", err)
db.res.Destroy()

if err := db.recreatePool(ctx); err != nil {
db.res.Destroy()
return
}
}
db.logger.Debug("DBWithPool: release done with cleanup", "dbName", db.dbName)
}
Expand All @@ -69,3 +71,18 @@ func (db *DBWithPool) release() {
func (db *DBWithPool) Pool() *pgxpool.Pool {
return db.res.Value().pool
}

func (db *DBWithPool) recreatePool(ctx context.Context) error {
db.logger.Debug("DBWithPool: recreatePool called", "dbName", db.dbName)
db.Pool().Close()

newPgxPool, err := pgxpool.NewWithConfig(ctx, db.res.Value().config)
if err != nil {
db.res.Destroy()
db.logger.Error("DBWithPool: recreatePool failed", "dbName", db.dbName, "err", err)
return err
}
db.logger.Debug("DBWithPool: recreatePool succeeded", "dbName", db.dbName)
db.res.Value().pool = newPgxPool
return nil
}
1 change: 1 addition & 0 deletions internal/testdb/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func (m *Manager) allocatePool(ctx context.Context) (*poolWithDBName, error) {
if m.cleanup != nil {
m.logger.Debug("DBManager: allocatePool calling cleanup", "dbName", dbName)
if err := m.cleanup(ctx, pgxp); err != nil {
m.logger.Error("DBManager: error during allocatePool cleanup", "error", err)
pgxp.Close()
return nil, fmt.Errorf("error during cleanup: %w", err)
}
Expand Down

0 comments on commit bacf8b5

Please sign in to comment.