atc: Consider image volumes in volume-locality strategy #9188
Conversation
There was a problem hiding this comment.
I'm not sure this is an assumption we want to bake into the placement strategy. We're assuming that the image artifact is always the volume that takes up the most disk space. We know this is not always the case though.
I'd be open to the latter change this PR makes though, where if there are no other inputs then the image weight is taken into account. That makes sense for all containers in this situation.
|
So you're saying remove the additional (5) weight for the image artifact? |
|
Yeah dropping the whole Also realized that the image was never even considered as a volume for this strategy. That alone will be a nice improvement to the strategy. |
|
The weight was requested on the original PR by @evanchaoli |
|
Ah, no one ever reviewed that PR. Evan was also never a maintainer. My feedback is the same. I'd also like for Andy to get credit for the work done by him. Updating commit message to include him as Co-Author. |
Rebase of concourse#8057 Often the largest volume that may need streaming is the image that the container executes in. By considering this volume too, this may allow for slightly better placement/reduced streaming for some builds (in particular builds with no inputs or caches will now prefer to be on the same worker as their image volume rather than a random one) Signed-off-by: Mathias Bogaert <mathias.bogaert@gmail.com> Co-Authored-by: Andy Paine <andy.paine@engineerbetter.com>
See #8057
Often the largest volume that may need streaming is the image that the container executes in. By considering this volume too, this may allow for slightly better placement/reduced streaming for some builds (in particular builds with no inputs or caches will now prefer to be on the same worker as their image volume rather than a random one)