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

Anchor input fetches to source action id #11236

Closed
wants to merge 8 commits into from

Conversation

werkt
Copy link
Contributor

@werkt werkt commented Apr 27, 2020

Provide the specific action id via RequestMetadata which provided an
action input artifact when using remote_download_minimal. This replaces
the unattributable "fetch-remote-inputs" identifier populated for each
input via a nested context.

Provide the specific action id via RequestMetadata which provided an
action input artifact when using remote_download_minimal. This replaces
the unattributable "fetch-remote-inputs" identifier populated for each
input via a nested context.
@aiuto aiuto requested review from michajlo and meisterT April 28, 2020 14:11
@aiuto aiuto added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Apr 28, 2020
@meisterT
Copy link
Member

cc @ulfjack

@meisterT
Copy link
Member

This seems to fail on import: https://buildkite.com/bazel/google-bazel-presubmit/builds/33753#_

I'll have a closer look tomorrow

@werkt
Copy link
Contributor Author

werkt commented Apr 28, 2020

This seems to fail on import: https://buildkite.com/bazel/google-bazel-presubmit/builds/33753#_

Just needs a merge update. Forthcoming.

@werkt
Copy link
Contributor Author

werkt commented Apr 29, 2020

Ping on this, should be clean for merge.

@meisterT
Copy link
Member

Sorry George, this needs some changes to internal code. I'll try to fix it next week.

@bazel-io bazel-io closed this in 3ef8fb9 May 6, 2020
@coeuvre
Copy link
Member

coeuvre commented Oct 18, 2022

@werkt We are considering removing action_id from remote metadata. Can you elaborate more why would you like to track input prefetches with source action_id?

According to the REAPI spec, action_id is

An identifier that ties multiple requests to the same action. For example, multiple requests to the CAS, Action Cache, and Execution API are used in order to compile foo.cc.

The input prefetches are requested by local actions that depend on outputs of remote executed action (which is identified by the action_id). These requests are not part of generating action hence shouldn't be bound with the action_id.

cc @tjgq

copybara-service bot pushed a commit that referenced this pull request Feb 17, 2023
actionId was added by #11236 to track download requests for input prefetches.

However, according to the REAPI spec, action_id is

> An identifier that ties multiple requests to the same action. For example, multiple requests to the CAS, Action Cache, and Execution API are used in order to compile foo.cc.

The input prefetches are requested by local actions that depend on outputs of remote executed action (which is identified by the action_id). These requests are not part of generating action hence shouldn't be bound with the action_id.

We could include ActionExecutionMetadata of the local action for the requests for the tracking purpose, but that desires another CL.

PiperOrigin-RevId: 510430853
Change-Id: I8d64a4f7033320a394e02a2b11498f09a3d15310
tjgq pushed a commit to tjgq/bazel that referenced this pull request Mar 10, 2023
actionId was added by bazelbuild#11236 to track download requests for input prefetches.

However, according to the REAPI spec, action_id is

> An identifier that ties multiple requests to the same action. For example, multiple requests to the CAS, Action Cache, and Execution API are used in order to compile foo.cc.

The input prefetches are requested by local actions that depend on outputs of remote executed action (which is identified by the action_id). These requests are not part of generating action hence shouldn't be bound with the action_id.

We could include ActionExecutionMetadata of the local action for the requests for the tracking purpose, but that desires another CL.

PiperOrigin-RevId: 510430853
Change-Id: I8d64a4f7033320a394e02a2b11498f09a3d15310
ShreeM01 pushed a commit that referenced this pull request Mar 10, 2023
actionId was added by #11236 to track download requests for input prefetches.

However, according to the REAPI spec, action_id is

> An identifier that ties multiple requests to the same action. For example, multiple requests to the CAS, Action Cache, and Execution API are used in order to compile foo.cc.

The input prefetches are requested by local actions that depend on outputs of remote executed action (which is identified by the action_id). These requests are not part of generating action hence shouldn't be bound with the action_id.

We could include ActionExecutionMetadata of the local action for the requests for the tracking purpose, but that desires another CL.

PiperOrigin-RevId: 510430853
Change-Id: I8d64a4f7033320a394e02a2b11498f09a3d15310

Co-authored-by: Googler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants