Add min-idle time for server.

the autoscaler will shutdown any server that is not busy but has reached the min_age parameter (30m currently)

This feature adds a new flag that basically resets the age to 0 every time a new build is executed.

This will allow us to decrease the min-age but allow the servers to be used for a longer period of time during busy hours, thus avoiding starting up too many servers and decreasing the build startup times

This is a feature added by a contributor to the original autoscaler code. harness is very slow to accept contributions so the PR has sat there for about 1 month. drone#125

Author: bsda

config/config.go

@@ -32,9 +32,10 @@ type (
 		}
 
 		Pool struct {
-			Min    int           `default:"2"`
-			Max    int           `default:"4"`
-			MinAge time.Duration `default:"55m" split_words:"true"`
+			Min     int           `default:"2"`
+			Max     int           `default:"4"`
+			MinAge  time.Duration `default:"55m" split_words:"true"`
+			MinIdle time.Duration `default:"0" split_words:"true"`
 		}
 
 		Check struct {

config/load_test.go

@@ -35,6 +35,9 @@ func TestDefaults(t *testing.T) {
 	if got, want := conf.Pool.MinAge, time.Minute*55; got != want {
 		t.Errorf("Want default DRONE_POOL_MIN_AGE of %d, got %d", want, got)
 	}
+	if got, want := conf.Pool.MinIdle, time.Minute*0; got != want {
+		t.Errorf("Want default DRONE_POOL_MIN_IDLE of %d, got %d", want, got)
+	}
 
 	if got, want := conf.Check.Interval, time.Minute; got != want {
 		t.Errorf("Want default DRONE_INSTALL_CHECK_INTERVAL of %s, got %s", want, got)
@@ -74,6 +77,7 @@ func TestLoad(t *testing.T) {
 		"DRONE_LOGS_PRETTY":                "true",
 		"DRONE_CAPACITY_BUFFER":            "3",
 		"DRONE_POOL_MIN_AGE":               "1h",
+		"DRONE_POOL_MIN_IDLE":              "15m",
 		"DRONE_POOL_MIN":                   "1",
 		"DRONE_POOL_MAX":                   "5",
 		"DRONE_SERVER_HOST":                "drone.company.com",
@@ -205,7 +209,8 @@ var jsonConfig = []byte(`{
   "Pool": {
     "Min": 1,
     "Max": 5,
-    "MinAge": 3600000000000
+    "MinAge": 3600000000000,
+    "MinIdle": 900000000000
   },
   "Server": {
     "Host": "drone.company.com",

engine/engine.go

@@ -100,6 +100,7 @@ func New(
 			kernel:     config.Agent.Kernel,
 			buffer:     config.CapacityBuffer,
 			ttu:        config.Pool.MinAge,
+			tti:        config.Pool.MinIdle,
 			min:        config.Pool.Min,
 			max:        config.Pool.Max,
 			cap:        config.Agent.Concurrency,

engine/planner.go

@@ -30,6 +30,7 @@ type planner struct {
 	cap        int           // capacity per-server
 	buffer     int           // buffer capacity to have warm and ready
 	ttu        time.Duration // minimum server age
+	tti        time.Duration // minimum server idle time
 	labels     map[string]string
 
 	client  drone.Client
@@ -76,6 +77,16 @@ func (p *planner) Plan(ctx context.Context) error {
 
 	ctx = logger.WithContext(ctx, log)
 
+	// if MinIdle is being used, track busy servers
+	if p.tti > 0 {
+		_, err = p.updateBusy(ctx)
+		if err != nil {
+			log.WithError(err).
+				Errorln("cannot check for busy servers")
+			return err
+		}
+	}
+
 	free := max(capacity-running-p.buffer, 0)
 	diff := serverDiff(pending, free, p.cap)
 
@@ -104,6 +115,45 @@ func (p *planner) Plan(ctx context.Context) error {
 	return nil
 }
 
+
+// helper function checks for busy running instances and updates idle timer
+func (p *planner) updateBusy(ctx context.Context) (count int, err error) {
+	logger := logger.FromContext(ctx)
+
+	servers, err := p.servers.ListState(ctx, autoscaler.StateRunning)
+	if err != nil {
+		logger.WithError(err).
+			Errorln("cannot fetch server list")
+		return count, err
+	}
+
+	// check for busy servers to update idle timers
+	busy, err := p.listBusy(ctx)
+	if err != nil {
+		logger.WithError(err).
+			Errorln("cannot ascertain busy server list")
+		return count, err
+	}
+
+	for _, server := range servers {
+		if _, ok := busy[server.Name]; ok {
+			err := p.servers.Busy(ctx, server)
+			if err != nil {
+				logger.WithError(err).
+					WithField("server", server.Name).
+					WithField("updated", server.Updated).
+					Errorln("cannot update server as busy")
+			}
+			logger.WithField("server", server.Name).
+				Debugln("updated busy server")
+			count++
+		}
+	}
+	logger.Debugf("found %d busy servers", count)
+	return count, nil
+}
+
+
 // helper function allocates n new server instances.
 func (p *planner) alloc(ctx context.Context, n int) error {
 	logger := logger.FromContext(ctx)
@@ -186,6 +236,16 @@ func (p *planner) mark(ctx context.Context, n int) error {
 			continue
 		}
 
+		// skip servers that have not reached a min idle time
+		if time.Now().Before(time.Unix(server.LastBusy, 0).Add(p.tti)) {
+			logger.
+				WithField("server", server.Name).
+				WithField("idle", timeDiff(time.Now(), time.Unix(server.LastBusy, 0))).
+				WithField("min-idle", p.tti).
+				Debugln("server min-idle not reached")
+			continue
+		}
+
 		idle = append(idle, server)
 		logger.WithField("server", server.Name).
 			Debugln("server is idle")

engine/planner_test.go

@@ -399,6 +399,51 @@ func TestScale_MinAge(t *testing.T) {
 	}
 }
 
+// This test verifies that idle servers are not
+// garbage collected until the min-idle is reached.
+func TestScale_MinIdle(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	// x2 capacity
+	servers := []*autoscaler.Server{
+		{Name: "server1", Capacity: 1, State: autoscaler.StateRunning, Created: time.Now().Unix(), LastBusy: time.Now().Add(time.Minute * 10 * -1).Unix()},
+		{Name: "server2", Capacity: 1, State: autoscaler.StateRunning, Created: time.Now().Unix(), LastBusy: time.Now().Add(time.Minute * 60 * -1).Unix()},
+	}
+
+	// x0 running builds
+	// x0 pending builds
+	builds := []*drone.Stage{}
+
+	store := mocks.NewMockServerStore(controller)
+	store.EXPECT().List(gomock.Any()).Return(servers, nil)
+	store.EXPECT().ListState(gomock.Any(), autoscaler.StateRunning).Return(servers, nil)
+	store.EXPECT().ListState(gomock.Any(), autoscaler.StateRunning).Return(servers, nil)
+	// we should expect a call to shut down only server2 since the last busy
+	// time of 1 hour ago satisfies the MinIdle (tti) of 30 minutes.
+	store.EXPECT().Update(gomock.Any(), servers[1]).Return(nil)
+
+	client := mocks.NewMockClient(controller)
+	client.EXPECT().Queue().Return(builds, nil)
+	client.EXPECT().Queue().Return(builds, nil)
+	client.EXPECT().Queue().Return(builds, nil)
+
+	p := planner{
+		cap:     2,
+		min:     1,
+		max:     4,
+		ttu:     0,
+		tti:     time.Minute * 30,
+		client:  client,
+		servers: store,
+	}
+
+	err := p.Plan(context.TODO())
+	if err != nil {
+		t.Error(err)
+	}
+}
+
 func TestPlan_ShutdownIdle(t *testing.T) {
 	controller := gomock.NewController(t)
 	defer controller.Finish()

mocks/mock_server.go

@@ -52,6 +52,9 @@ type ServerStore interface {
 	// Update the server record in the store.
 	Update(context.Context, *Server) error
 
+	// Update the server record that it is busy.
+	Busy(context.Context, *Server) error
+
 	// Delete the server record from the store.
 	Delete(context.Context, *Server) error
 
@@ -81,4 +84,5 @@ type Server struct {
 	Updated  int64        `db:"server_updated"  json:"updated"`
 	Started  int64        `db:"server_started"  json:"started"`
 	Stopped  int64        `db:"server_stopped"  json:"stopped"`
+	LastBusy int64        `db:"server_lastbusy" json:"lastbusy"`
 }

server.go

@@ -20,6 +20,10 @@ var migrations = []struct {
 		name: "create-index-server-state",
 		stmt: createIndexServerState,
 	},
+	{
+		name: "alter-table-servers-add-column-server-lastbusy",
+		stmt: alterTableServersAddColumnServerLastbusy,
+	},
 }
 
 // Migrate performs the database migration. If the migration fails
@@ -131,3 +135,11 @@ CREATE INDEX ix_servers_id ON servers (server_id);
 var createIndexServerState = `
 CREATE INDEX ix_servers_state ON servers (server_state);
 `
+
+//
+// 002_add_column_lastbusy.sql
+//
+
+var alterTableServersAddColumnServerLastbusy = `
+ALTER TABLE servers ADD COLUMN server_lastbusy INTEGER NOT NULL DEFAULT 0;
+`

store/migrate/mysql/ddl_gen.go

@@ -20,6 +20,10 @@ var migrations = []struct {
 		name: "create-index-server-state",
 		stmt: createIndexServerState,
 	},
+	{
+		name: "alter-table-servers-add-column-server-lastbusy",
+		stmt: alterTableServersAddColumnServerLastbusy,
+	},
 }
 
 // Migrate performs the database migration. If the migration fails
@@ -131,3 +135,11 @@ CREATE INDEX ix_servers_id ON servers (server_id);
 var createIndexServerState = `
 CREATE INDEX ix_servers_state ON servers (server_state);
 `
+
+//
+// 002_add_column_lastbusy.sql
+//
+
+var alterTableServersAddColumnServerLastbusy = `
+ALTER TABLE servers ADD COLUMN server_lastbusy INTEGER NOT NULL DEFAULT 0;
+`

store/migrate/postgres/ddl_gen.go

@@ -20,6 +20,10 @@ var migrations = []struct {
 		name: "create-index-server-state",
 		stmt: createIndexServerState,
 	},
+	{
+		name: "alter-table-servers-add-column-server-lastbusy",
+		stmt: alterTableServersAddColumnServerLastbusy,
+	},
 }
 
 // Migrate performs the database migration. If the migration fails
@@ -131,3 +135,11 @@ CREATE INDEX IF NOT EXISTS ix_servers_id ON servers (server_id);
 var createIndexServerState = `
 CREATE INDEX IF NOT EXISTS ix_servers_state ON servers (server_state);
 `
+
+//
+// 002_add_column_lastbusy.sql
+//
+
+var alterTableServersAddColumnServerLastbusy = `
+ALTER TABLE servers ADD COLUMN server_lastbusy INTEGER NOT NULL DEFAULT 0;
+`