don't lock while extending, reuse recreateDatabaseGracefully

pkg/manager/manager_test.go

@@ -822,19 +822,28 @@ func TestManagerReturnTestDatabase(t *testing.T) {
 	// finally return it
 	assert.NoError(t, m.ReturnTestDatabase(ctx, hash, testDB1.ID))
 
+	// regetting these databases is quite random. Let's try to get the same id again...
 	// on first GET call the pool has been extended
 	// we will get the newly created DB
 	testDB2, err := m.GetTestDatabase(ctx, hash)
 	assert.NoError(t, err)
-	assert.NotEqual(t, testDB1.ID, testDB2.ID)
 
 	// next in 'ready' channel should be the returned DB
 	testDB3, err := m.GetTestDatabase(ctx, hash)
 	assert.NoError(t, err)
-	assert.Equal(t, testDB1.ID, testDB3.ID)
+
+	// restored db
+	var targetConnectionString string
+	if testDB2.ID == testDB1.ID {
+		targetConnectionString = testDB2.Config.ConnectionString()
+	} else if testDB3.ID == testDB1.ID {
+		targetConnectionString = testDB3.Config.ConnectionString()
+	} else {
+		t.Fatal("We should have been able to get the previously returned database.")
+	}
 
 	// assert that it hasn't been cleaned but just reused directly
-	db, err = sql.Open("postgres", testDB3.Config.ConnectionString())
+	db, err = sql.Open("postgres", targetConnectionString)
 	require.NoError(t, err)
 	require.NoError(t, db.PingContext(ctx))
 

pkg/pool/pool.go

@@ -574,20 +574,19 @@ func (pool *HashPool) extend(ctx context.Context) error {
 
 	reg := trace.StartRegion(ctx, "worker_wait_for_lock_hash_pool")
 	pool.Lock()
-	defer pool.Unlock()
-
 	reg.End()
 
 	// get index of a next test DB - its ID
 	index := len(pool.dbs)
 	if index == cap(pool.dbs) {
 		log.Error().Int("dbs", len(pool.dbs)).Int("cap", cap(pool.dbs)).Err(ErrPoolFull).Msg("pool is full")
+		pool.Unlock()
 		return ErrPoolFull
 	}
 
-	// initalization of a new DB using template config
+	// initalization of a new DB using template config, it must start in state dirty!
 	newTestDB := existingDB{
-		state: dbStateReady,
+		state: dbStateDirty,
 		TestDatabase: db.TestDatabase{
 			Database: db.Database{
 				TemplateHash: pool.templateDB.TemplateHash,
@@ -599,24 +598,15 @@ func (pool *HashPool) extend(ctx context.Context) error {
 	// set DB name
 	newTestDB.Database.Config.Database = makeDBName(pool.TestDBNamePrefix, pool.templateDB.TemplateHash, index)
 
-	log.Trace().Int("id", index).Msg("adding...")
-
-	reg = trace.StartRegion(ctx, "worker_db_operation")
-	err := pool.recreateDB(ctx, &newTestDB)
-	reg.End()
-
-	if err != nil {
-		return err
-	}
-
-	// add new test DB to the pool
+	// add new test DB to the pool (currently it's dirty!)
 	pool.dbs = append(pool.dbs, newTestDB)
 
-	pool.ready <- newTestDB.ID
-
+	log.Trace().Int("id", index).Msg("appended as dirty, recreating...")
 	pool.unsafeTraceLogStats(log)
+	pool.Unlock()
 
-	return nil
+	// forced recreate...
+	return pool.recreateDatabaseGracefully(ctx, index)
 }
 
 func (pool *HashPool) RemoveAll(ctx context.Context, removeFunc RemoveDBFunc) error {