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.

Sign up for GitHub

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

Jump to bottom

Don't upload large files to remote-cache #1856

Merged
merged 1 commit into from
May 21, 2021
Merged

Don't upload large files to remote-cache #1856

merged 1 commit into from
May 21, 2021

Conversation

alexeagle
Copy link
Collaborator

Fixes #1855

@google-cla google-cla bot added the cla: yes label May 20, 2021
gravypod
gravypod requested changes May 20, 2021
View reviewed changes
Copy link
Collaborator

@gravypod gravypod left a comment

Choose a reason for hiding this comment

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

Looks good.

skylib/zip.bzl Show resolved Hide resolved
@alexeagle alexeagle merged commit 92d87b0 into bazelbuild:master May 21, 2021
alexeagle added a commit to alexeagle/rules_docker that referenced this pull request May 24, 2021
@alexeagle
More fixes for actions that shouldn't upload to remote-cache
412b91c 
Similar to bazelbuild#1856. I found these by patching that PR and running through a big monorepo and searching execution log for more remote cache uploads over 10MB
@alexeagle alexeagle mentioned this pull request May 24, 2021
Merged
alexeagle added a commit to alexeagle/rules_docker that referenced this pull request May 24, 2021
@alexeagle
More fixes for actions that shouldn't upload to remote-cache
4217606 
Similar to bazelbuild#1856. I found these by patching that PR and running through a big monorepo and searching execution log for more remote cache uploads over 10MB
alexeagle added a commit that referenced this pull request May 24, 2021
@alexeagle
More fixes for actions that shouldn't upload to remote-cache
e33dda0 
Similar to #1856. I found these by patching that PR and running through a big monorepo and searching execution log for more remote cache uploads over 10MB
@codersasha
Copy link

Is there a way to disable this? We really do want to use remote-cache for our bazel Docker builds, our cache is fast and building locally each time takes way too long.

@alexeagle
Copy link
Collaborator Author

Which action is slow? Join_layers? Maybe we should just revert that one.

@codersasha
Copy link

codersasha commented Mar 8, 2022 •
edited
Loading

Yes. I think reverting that one is a good idea. Our bazel cache is a giant bucket and we'd prefer not to rebuild anything if possible; can't see a way to force the join_layers to cache if this flag is set. At the very least, you could always add the tag no-remote-cache to the layer rules to not cache them.

I also noticed that the apt rules also have remote-cache disabled, these are also slow and should also be cached IMO.

Either that, or some way to prevent the rebuild of container_image when it is made up of layers built with container_run_and_commit_layer (and hence, aren't pushed to a remote cache).

Thanks for the prompt response.

alexeagle added a commit that referenced this pull request Mar 16, 2022
@alexeagle
perf: remove no-remote-cache execution requirements
fad444a 
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.
alexeagle added a commit that referenced this pull request Mar 16, 2022
@alexeagle
perf: remove no-remote-cache execution requirements
f167a30 
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.
@alexeagle alexeagle mentioned this pull request Mar 16, 2022
Merged
gravypod pushed a commit that referenced this pull request Mar 16, 2022
@alexeagle
perf: remove no-remote-cache execution requirements (#2043)
094a012 
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.
@codersasha
Copy link

Thank you! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gravypod gravypod gravypod approved these changes

@pcj pcj Awaiting requested review from pcj pcj is a code owner

@smukherj1 smukherj1 Awaiting requested review from smukherj1 smukherj1 is a code owner

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

container rules put high load on remote cache
3 participants
@alexeagle @codersasha @gravypod