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

perf: remove no-remote-cache execution requirements #2043

Merged
merged 1 commit into from
Mar 16, 2022
Merged

Conversation

alexeagle
Copy link
Collaborator

These were introduced to reduce load on a remote-cache instance to avoid network saturation.
A month later, a feature was added in one remote-cache implementatation which provides a different fix:
buchgr/bazel-remote#440 rejects large input files on upload.

In practice, while these action do often produce huge outputs, they are also slow to re-execute.
In many cases it's worth it to use a remote-cache for RunAndCommitLayer in particular to avoid a local rebuild
even though it's a large network fetch.
Currently users can't configure this because we've hardcoded the values. If they do want to keep the
no-remote-cache execution requirement, they can do this via a tag (provided they opt-in to
experimental_allow_tags_propagation, see bazelbuild/bazel#8830)

#1856 (comment) is an example of a user
asking for these to be removed.

These were introduced to reduce load on a remote-cache instance to avoid network saturation.
A month later, a feature was added in one remote-cache implementatation which provides a different fix:
buchgr/bazel-remote#440 rejects large input files on upload.

In practice, while these action do often produce huge outputs, they are also slow to re-execute.
In many cases it's worth it to use a remote-cache for RunAndCommitLayer in particular to avoid a local rebuild
even though it's a large network fetch.
Currently users can't configure this because we've hardcoded the values. If they do want to keep the
no-remote-cache execution requirement, they can do this via a tag (provided they opt-in to
experimental_allow_tags_propagation, see bazelbuild/bazel#8830)

#1856 (comment) is an example of a user
asking for these to be removed.
@gravypod gravypod merged commit 094a012 into master Mar 16, 2022
@sluongng
Copy link
Contributor

It might be beneficial for us to document --modify_execution_info= flag to be the work around for folks who would still need this feature.

@tbrethersd
Copy link

@sluongng Just wanted to show my interest in hearing the --modify_execution_info= workaround and would it be possible to use this prior to a release to achieve the results of this PR?

@sluongng
Copy link
Contributor

yes, using --modify_execution_info you can override the execution requirement per mnemonic. So you should be able to add/remove the no-remote-cache for each mnemonic that was modified in this PR.

@nkoroste
Copy link

Which mnemonic(s) are you guys using for this?

@alexeagle
Copy link
Collaborator Author

I'll begin by pointing out that rules_docker has some actions which are doing The Wrong Thing by reading very large inputs and writing very large outputs. rules_oci is a better alternative because we only run actions that only ever operate on a single layer at a time, no "flattening" operations which cause this problem. For example, there should never be an action that takes a base layer as an input, it should just have its digest shuttled around between metadata files.

@nkoroste the relevant mnemonics for rules_docker actions that often have huge inputs/outputs are:

GZIP
GUNZIP
RunAndCommitLayer
ImageLayer
JoinLayers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants