Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore cached input from volume-locality's consideration #8061

Merged
merged 2 commits into from
Feb 22, 2022

Conversation

evanchaoli
Copy link
Contributor

@evanchaoli evanchaoli commented Feb 12, 2022

Changes proposed by this PR

Closes #8070

  • Ignore cached input from volume-locality's Order()

Notes to reviewer

Release Note

  • When EnableCacheStreamedVolume is enabled and container placement strategy is volume-locality, as get step may not fetch a resource if the resource is found in cache, following step containers may all be placed to the worker where cached resource is found. That worker might be overloaded when there are other workers available. This PR fixes the problem.

@evanchaoli evanchaoli requested a review from a team as a code owner February 12, 2022 08:48
@evanchaoli evanchaoli added this to the v7.7.0 milestone Feb 12, 2022
@evanchaoli evanchaoli force-pushed the no-input-strategy branch 2 times, most recently from 928f2e0 to 260b0ec Compare February 14, 2022 07:18
@xtremerui xtremerui removed this from the v7.7.0 milestone Feb 14, 2022
@navdeep-pama navdeep-pama added this to the v7.8.0 milestone Feb 14, 2022
@evanchaoli evanchaoli force-pushed the no-input-strategy branch 3 times, most recently from d4d6b1d to 084ff5c Compare February 16, 2022 03:38
Signed-off-by: Evan <chaol@vmware.com>
@evanchaoli evanchaoli changed the title Add noinput-container-placement-strategy. Ignore cached input from volume-locality's considration Feb 16, 2022
@evanchaoli evanchaoli changed the title Ignore cached input from volume-locality's considration Ignore cached input from volume-locality's consideration Feb 16, 2022
@evanchaoli
Copy link
Contributor Author

@xtremerui I have reduced scope of this PR to focus on "ignore cached input from volume-locality's consideration". Hope that helps quicker review.

}
})

It("runs with specified inputs", func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is a specified input related to cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a typo. Goal of this test is to make sure when only input is from cache, container spec is well populated, FromCache should be true.

Signed-off-by: Evan <chaol@vmware.com>
@xtremerui
Copy link
Contributor

In acceptance test, the task unit is placed in expected workers in first 3 builds while the deployment has 3 workers. In build 4, it reuse a worker with local volume of input and task image so no streaming.

@xtremerui xtremerui added the release/documented Documentation and release notes have been updated. label Feb 22, 2022
@xtremerui xtremerui merged commit 4db49eb into concourse:master Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement release/documented Documentation and release notes have been updated.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Volume-locality strategy should ignore inputs of cached resource
3 participants