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

Take the no-remote-exec tag into account when computing the action salt #17304

Conversation

EdSchouten
Copy link
Contributor

@EdSchouten EdSchouten commented Jan 24, 2023

Multiple times have I noticed build breakages, where people add/remove the "no-remote-exec" annotation from targets, which only came to light after existing remote cache entries expire.

For example, people may try to promote a test to run remotely. "bazel test" shows the test passes, so they assume the test is safe to run remotely. Later on, the cache entry for that test expires. Or the test changes, causing its action cache key to be different. The test gets rebuilt remotely. This now fails, because the test never succeeded remotely to begin with. The cache entry belonged to the action that ran locally.

Let's formalize the schema of the "salt" field. Instead of reusing the Platform message, let's declare our own message that we can use to add arbitrary properties that should be taken into account when determining the action's uniqueness.

@EdSchouten EdSchouten force-pushed the eschouten/20230124-may-be-executed-remotely branch 6 times, most recently from 5f10968 to c040120 Compare January 24, 2023 18:35
Multiple times have I noticed build breakages, where people add/remove
the "no-remote-exec" annotation from targets, which only came to light
after existing remote cache entries expire.

For example, people may try to promote a test to run remotely. "bazel
test" shows the test passes, so they assume the test is safe to run
remotely. Later on, the cache entry for that test expires. Or the test
changes, causing its action cache key to be different. The test gets
rebuilt remotely. This now fails, because the test never succeeded
remotely to begin with.

Let's formalize the schema of the "salt" field. Instead of reusing the
Platform message, let's declare our own message that we can use to add
arbitrary properties that should be taken into account when determining
the action's uniqueness.
@EdSchouten EdSchouten force-pushed the eschouten/20230124-may-be-executed-remotely branch from c040120 to f2e36c4 Compare January 24, 2023 18:59
@EdSchouten EdSchouten marked this pull request as ready for review January 24, 2023 19:35
@EdSchouten EdSchouten requested a review from a team as a code owner January 24, 2023 19:35
@EdSchouten EdSchouten requested review from coeuvre and tjgq January 24, 2023 20:15
@sgowroji sgowroji added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Jan 25, 2023
@EdSchouten EdSchouten changed the title Take the no-remote-exec flag into account when computing the action salt Take the no-remote-exec tag into account when computing the action salt Jan 25, 2023
@coeuvre coeuvre added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jan 25, 2023
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jan 27, 2023
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
Multiple times have I noticed build breakages, where people add/remove the "no-remote-exec" annotation from targets, which only came to light after existing remote cache entries expire.

For example, people may try to promote a test to run remotely. "bazel test" shows the test passes, so they assume the test is safe to run remotely. Later on, the cache entry for that test expires. Or the test changes, causing its action cache key to be different. The test gets rebuilt remotely. This now fails, because the test never succeeded remotely to begin with. The cache entry belonged to the action that ran locally.

Let's formalize the schema of the "salt" field. Instead of reusing the Platform message, let's declare our own message that we can use to add arbitrary properties that should be taken into account when determining the action's uniqueness.

Closes #17304.

PiperOrigin-RevId: 505010966
Change-Id: Ib88e769795f18b1724895ebd79835b2bc3b13e1e
tjgq pushed a commit to tjgq/bazel that referenced this pull request Sep 8, 2023
…ction salt

Multiple times have I noticed build breakages, where people add/remove the "no-remote-exec" annotation from targets, which only came to light after existing remote cache entries expire.

For example, people may try to promote a test to run remotely. "bazel test" shows the test passes, so they assume the test is safe to run remotely. Later on, the cache entry for that test expires. Or the test changes, causing its action cache key to be different. The test gets rebuilt remotely. This now fails, because the test never succeeded remotely to begin with. The cache entry belonged to the action that ran locally.

Let's formalize the schema of the "salt" field. Instead of reusing the Platform message, let's declare our own message that we can use to add arbitrary properties that should be taken into account when determining the action's uniqueness.

Closes bazelbuild#17304.

PiperOrigin-RevId: 505010966
Change-Id: Ib88e769795f18b1724895ebd79835b2bc3b13e1e
iancha1992 pushed a commit that referenced this pull request Sep 8, 2023
…ction salt (#19457)

Multiple times have I noticed build breakages, where people add/remove
the "no-remote-exec" annotation from targets, which only came to light
after existing remote cache entries expire.

For example, people may try to promote a test to run remotely. "bazel
test" shows the test passes, so they assume the test is safe to run
remotely. Later on, the cache entry for that test expires. Or the test
changes, causing its action cache key to be different. The test gets
rebuilt remotely. This now fails, because the test never succeeded
remotely to begin with. The cache entry belonged to the action that ran
locally.

Let's formalize the schema of the "salt" field. Instead of reusing the
Platform message, let's declare our own message that we can use to add
arbitrary properties that should be taken into account when determining
the action's uniqueness.

Closes #17304.

PiperOrigin-RevId: 505010966
Change-Id: Ib88e769795f18b1724895ebd79835b2bc3b13e1e

Co-authored-by: Ed Schouten <[email protected]>
@iancha1992
Copy link
Member

The changes in this PR have been included in Bazel 6.4.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc.
Thanks!

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