drone / autoscaler

Automatically adds or removes instances based on build volume
https://autoscale.drone.io
Other
193 stars 89 forks source link

Correct planner's behaviour when there are no capacity left for buffer #94

Open phantasia15 opened 3 years ago

phantasia15 commented 3 years ago

Hi, I'm currently using this configuration: DRONE_AGENT_CONCURRENCY=1, DRONE_CAPACITY_BUFFER=1. When there is only 1 server, 1 running build, I expect the autoscaler to scale the num of servers to 2 but the log say that no capacity changes required.

I took a look the source code and found this: https://github.com/drone/autoscaler/blob/290741f7c495d0cb0ca029e8ff0863586df99c43/engine/planner.go#L78 In the above case:

capacity = 1 (only 1 active server)
running = 1 (only 1 active build)
p.buffer = 1
-> free = max(1-1-1,0) = 0
-> diff = ceil(0-0)/1 = 0 
-> Since diff = 0, the autoscaler decides that no changes are required.

I think the problem is that buffer is included in free calculation, but free is forced to be at least 0, instead of -1 in this case. So I move the buffer into diff calcuation, treating it as a pending build: diff = serverDiff(pending+p.buffer, free, p.cap)). This fixes the above issue while still keeps the logic that free must be at least 0.

bradrydzewski commented 2 years ago

thanks, could you please add a unit test that re-creates the state required to reproduce, and that fails without this fix, but passes with the fix? If you look at the unit tests for this file we attempt to test all known states, so this state should definitely be included. Thanks!

phantasia15 commented 2 years ago

Hi, I've included the failed tests.