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

Add experimental_remote_cache_key_ignore_stamping to skip volatile stamping files on compute shared cache key #16240

Closed
wants to merge 1 commit into from

Conversation

bozaro
Copy link
Contributor

@bozaro bozaro commented Sep 8, 2022

Changes (enabled by --experimental_remote_cache_key_ignore_stamping flag):

  • added the --experimental_remote_cache_key_ignore_stamping flag, which removes volatile files from the cache key;
  • since without volatile files it is impossible to execute Action on build farm, Swap with volatile files is executes only locally.

As result, stamping spaws would execute locally, but with correct remote caching.

Related issues:

@bozaro bozaro force-pushed the stamping-on-shared-cache branch 3 times, most recently from 733b624 to 14bad35 Compare September 9, 2022 07:54
@bozaro bozaro marked this pull request as ready for review September 9, 2022 09:25
@bozaro bozaro requested a review from a team as a code owner September 9, 2022 09:25
@ShreeM01 ShreeM01 added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels Sep 12, 2022
@ShreeM01
Copy link
Contributor

Hi @bozaro! Could you please fix the build failures? Thanks!

@bozaro
Copy link
Contributor Author

bozaro commented Sep 30, 2022

I don't undestand this test and can't reproduce error locally:

Checking https://mirror.bazel.build/github.com/protocolbuffers/upb/archive/e5f26018368b11aab672e8e8bb76513f3620c579.tar.gz ...
curl: (7) Failed to connect to mirror.bazel.build port 443: Connection timed out
-- Test log: -----------------------------------------------------------
------------------------------------------------------------------------
test_verify_urls FAILED: Invalid urls: https://mirror.bazel.build/github.com/protocolbuffers/upb/archive/e5f26018368b11aab672e8e8bb76513f3620c579.tar.gz.
/b/f/w/bazel-out/k8-fastbuild/bin/src/test/shell/bazel/verify_workspace.runfiles/io_bazel/src/test/shell/bazel/verify_workspace:66: in call to test_verify_urls

Tear down:

@fmeum
Copy link
Collaborator

fmeum commented Sep 30, 2022

I don't undestand this test and can't reproduce error locally:

Checking https://mirror.bazel.build/github.com/protocolbuffers/upb/archive/e5f26018368b11aab672e8e8bb76513f3620c579.tar.gz ...
curl: (7) Failed to connect to mirror.bazel.build port 443: Connection timed out
-- Test log: -----------------------------------------------------------
------------------------------------------------------------------------
test_verify_urls FAILED: Invalid urls: https://mirror.bazel.build/github.com/protocolbuffers/upb/archive/e5f26018368b11aab672e8e8bb76513f3620c579.tar.gz.
/b/f/w/bazel-out/k8-fastbuild/bin/src/test/shell/bazel/verify_workspace.runfiles/io_bazel/src/test/shell/bazel/verify_workspace:66: in call to test_verify_urls

Tear down:

Looks like a flake due to a network timeout. You can rerun the tests by amending and force-pushing.

@bozaro bozaro force-pushed the stamping-on-shared-cache branch 2 times, most recently from 7458cfa to 026bb1c Compare September 30, 2022 19:53
@bozaro
Copy link
Contributor Author

bozaro commented Sep 30, 2022

Rebase to master fixed the tests.

…stamping files on compute shared cache key
@bozaro
Copy link
Contributor Author

bozaro commented Mar 31, 2023

Rebased to master

@bozaro
Copy link
Contributor Author

bozaro commented Mar 31, 2023

I use changes from this PR in patched bazel versions: 5.3.0, 5.3.2, 5.4.0, 6.1.1 (Linux binaries and per-version branches avalialbe here: https://github.com/joomcode/bazel/releases) and it works as expected.

@keith keith removed the request for review from a team March 31, 2023 16:19
@gregestren gregestren removed request for a team and gregestren April 3, 2023 19:23
@ted-xie ted-xie removed their request for review April 3, 2023 19:38
@oquenchil oquenchil removed their request for review April 4, 2023 08:56
@tjgq
Copy link
Contributor

tjgq commented Oct 20, 2023

Closing in favor of --experimental_remote_scrub_config (new in Bazel 7).

@tjgq tjgq closed this Oct 20, 2023
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Oct 20, 2023
@bozaro
Copy link
Contributor Author

bozaro commented Nov 7, 2023

Replace volatile check by --experimental_remote_scrubbing_config usage:

@bozaro
Copy link
Contributor Author

bozaro commented Jan 12, 2024

I finished migrating to Basel 7.0.0 + #20070 yesterday. We are using this version to build the production version. I am satisfied.

Now I see the situation as follows:

  • Before version 7.0.0, there was no way to use stamping together with a remote cache and a remote build (any Action with volatile-status.txt never gets cache hit). I got out of the situation using the patched version with Add experimental_remote_cache_key_ignore_stamping to skip volatile stamping files on compute shared cache key #16240 (I was building versions 5.4.0, 6.0.0, 6.1.1, 6.2.0, 6.3.2, 6.4.0).
  • Since version 7.0.0, it has been possible to use stamping with a remote cache via scrubbing. This is a big step in the right direction. But using srubbing with a remote build is prohibited, which I want to fix by this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants