Skip to content

Conversation

@fmeum
Copy link
Collaborator

@fmeum fmeum commented Jan 23, 2025

ea4ad30 added an action as context for prefetches with BwoB, but assumed that all callers would use the prefetcher for inputs to locally executed actions. However, since then, it has also been used to download outputs of completed actions that have been explicitly requested (e.g. as outputs of top-level targets or those matching the regex path patterns). This resulted in downloads with a prefetcher action ID and action details for the action that produced the file, rather than consumed it, resulting in confusing situations for observability tools.

This is fixed by separately tracking the reason for the fetch and using an action ID of requested when the action has the requested file as an output.

@fmeum fmeum requested a review from a team as a code owner January 23, 2025 12:33
@github-actions github-actions bot added team-Performance Issues for Performance teams team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Jan 23, 2025
@fmeum
Copy link
Collaborator Author

fmeum commented Jan 23, 2025

CC @coeuvre @tjgq, this is related to the discussion in #17724

@fmeum fmeum mentioned this pull request Jan 23, 2025
6 tasks
@fmeum
Copy link
Collaborator Author

fmeum commented Jan 23, 2025

@bazel-io fork 8.1.0

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 23, 2025

@bazel-io fork 7.5.0

ea4ad30 added an action as context for prefetches with BwoB, but assumed that all callers would use the prefetcher for inputs to locally executed actions. However, since then, it has also been used to download outputs of completed actions that have been explicitly requested (e.g. as outputs of top-level targets or those matching the regex path patterns). This resulted in downloads with a `prefetcher` action ID and action details for the action that produced the file, rather than consumed it, resulting in confusing situations for observability tools.

This is fixed by separately tracking the reason for the fetch and using an action ID of `requested` when the action has the requested file as an output.
@fmeum fmeum force-pushed the fix-prefetcher-reporting branch from 742d4d1 to c4f34e7 Compare January 23, 2025 12:37
Copy link
Member

@coeuvre coeuvre left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM, please fix the tests and I will import it.

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 23, 2025

Thanks! LGTM, please fix the tests and I will import it.

Thanks, should be fixed now

@fmeum fmeum requested a review from coeuvre January 23, 2025 13:04
@meteorcloudy meteorcloudy 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 24, 2025
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jan 24, 2025
@coeuvre
Copy link
Member

coeuvre commented Jan 24, 2025

During internal review, we changed action id "prefetcher" to "input" and "requested" to "output" because the latter are easier to understand. This is technically a breaking change, but anyone who is consuming this need to update their code anyway.

@fmeum fmeum deleted the fix-prefetcher-reporting branch January 24, 2025 18:32
fmeum added a commit to fmeum/bazel that referenced this pull request Jan 24, 2025
ea4ad30 added an action as context for prefetches with BwoB, but assumed that all callers would use the prefetcher for inputs to locally executed actions. However, since then, it has also been used to download outputs of completed actions that have been explicitly requested (e.g. as outputs of top-level targets or those matching the regex path patterns). This resulted in downloads with a `prefetcher` action ID and action details for the action that produced the file, rather than consumed it, resulting in confusing situations for observability tools.

This is fixed by separately tracking the reason for the fetch. Using an action ID of `output` when the action has the requested file as an output, and "input" when the action has the requested file as an input.

Closes bazelbuild#25040.

PiperOrigin-RevId: 719246746
Change-Id: Ib95ff65ba68112b1a38ab3022e5b1a19ef74cc9f
(cherry picked from commit 998e762)
fmeum added a commit to fmeum/bazel that referenced this pull request Jan 24, 2025
…etadata

ea4ad30 added an action as context for prefetches with BwoB, but assumed that all callers would use the prefetcher for inputs to locally executed actions. However, since then, it has also been used to download outputs of completed actions that have been explicitly requested (e.g. as outputs of top-level targets or those matching the regex path patterns). This resulted in downloads with a `prefetcher` action ID and action details for the action that produced the file, rather than consumed it, resulting in confusing situations for observability tools.

This is fixed by separately tracking the reason for the fetch. Using an action ID of `output` when the action has the requested file as an output, and "input" when the action has the requested file as an input.

Closes bazelbuild#25040.

PiperOrigin-RevId: 719246746
Change-Id: Ib95ff65ba68112b1a38ab3022e5b1a19ef74cc9f
(cherry picked from commit 998e762)
github-merge-queue bot pushed a commit that referenced this pull request Jan 27, 2025
…etadata (#25057)

ea4ad30 added an action as context for
prefetches with BwoB, but assumed that all callers would use the
prefetcher for inputs to locally executed actions. However, since then,
it has also been used to download outputs of completed actions that have
been explicitly requested (e.g. as outputs of top-level targets or those
matching the regex path patterns). This resulted in downloads with a
`prefetcher` action ID and action details for the action that produced
the file, rather than consumed it, resulting in confusing situations for
observability tools.

This is fixed by separately tracking the reason for the fetch. Using an
action ID of `output` when the action has the requested file as an
output, and "input" when the action has the requested file as an input.

Closes #25040.

PiperOrigin-RevId: 719246746
Change-Id: Ib95ff65ba68112b1a38ab3022e5b1a19ef74cc9f 
(cherry picked from commit 998e762)

RELNOTES: CAS requests made when Bazel downloads a blob with Build
without the Bytes enabled now provide metadata with an action ID of
`input` if the blob is downloaded as the input to a local action and
`output` if it is a requested action output.
github-merge-queue bot pushed a commit that referenced this pull request Jan 27, 2025
#25058)

…etadata

ea4ad30 added an action as context for
prefetches with BwoB, but assumed that all callers would use the
prefetcher for inputs to locally executed actions. However, since then,
it has also been used to download outputs of completed actions that have
been explicitly requested (e.g. as outputs of top-level targets or those
matching the regex path patterns). This resulted in downloads with a
`prefetcher` action ID and action details for the action that produced
the file, rather than consumed it, resulting in confusing situations for
observability tools.

This is fixed by separately tracking the reason for the fetch. Using an
action ID of `output` when the action has the requested file as an
output, and "input" when the action has the requested file as an input.

Closes #25040.

PiperOrigin-RevId: 719246746
Change-Id: Ib95ff65ba68112b1a38ab3022e5b1a19ef74cc9f (cherry picked from
commit 998e762)

RELNOTES: CAS requests made when Bazel downloads a blob with Build
without the Bytes enabled now provide metadata with an action ID of
input if the blob is downloaded as the input to a local action and
output if it is a requested action output.
@iancha1992
Copy link
Member

The changes in this PR have been included in Bazel 7.5.0 RC3. 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=7.5.0rc3. Thanks!

fmeum added a commit to fmeum/bazel that referenced this pull request Jan 29, 2025
ea4ad30 added an action as context for prefetches with BwoB, but assumed that all callers would use the prefetcher for inputs to locally executed actions. However, since then, it has also been used to download outputs of completed actions that have been explicitly requested (e.g. as outputs of top-level targets or those matching the regex path patterns). This resulted in downloads with a `prefetcher` action ID and action details for the action that produced the file, rather than consumed it, resulting in confusing situations for observability tools.

This is fixed by separately tracking the reason for the fetch. Using an action ID of `output` when the action has the requested file as an output, and "input" when the action has the requested file as an input.

Closes bazelbuild#25040.

PiperOrigin-RevId: 719246746
Change-Id: Ib95ff65ba68112b1a38ab3022e5b1a19ef74cc9f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Performance Issues for Performance teams 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