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

Change execution log proto to differentiate between remote and disk cache hits #8192

Closed
kastiglione opened this issue Apr 29, 2019 · 2 comments
Labels
P2 We'll consider working on this in future. (Assignee optional) stale Issues or PRs that are stale (no activity for 30 days) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request

Comments

@kastiglione
Copy link
Contributor

kastiglione commented Apr 29, 2019

Description of the bug and feature request:

The execution log protobuf contains remote_cache_hit, but does not have an equivalent disk_cache_hit.

There appears to also be a bug here. @keith found that remote_cache_hit is being set to true for disk cache hits. For the purpose of metrics, these should be split out, as the times will be different.

Feature requests: what underlying problem are you trying to solve with this feature?

We want to track performance of caching, including cache hit ratios and time to fetch from cache. The metrics would be more precise if the different types of cache hits were kept distinct.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Build twice, once to populate the disk cache, once to use the cache and generate an execution log.

  1. Populate disk cache: --disk_cache=some/path --remote_upload_local_results
  2. Produce execution log: --disk_cache=some/path --experimental_execution_log_file=path/to/execution.log

Then, inspect the execution log to see that remote_cache_hit is true.

What operating system are you running Bazel on?

macos

What's the output of bazel info release?

0.25.0rc2

Any other information, logs, or outputs that you want to share?

This relates to the recent work to support using both disk and remote http cache together. See #7512, #8052, and #8141.

@ishikhman ishikhman added team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request untriaged labels Apr 30, 2019
@buchgr buchgr added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels May 2, 2019
@kastiglione kastiglione changed the title Change build event proto to differentiate between remote and disk cache hits Change execution log proto to differentiate between remote and disk cache hits May 6, 2019
@kastiglione
Copy link
Contributor Author

Update: I had myself confused, this is the execution log not the build event log. I've updated the issue accordingly.

@ishikhman ishikhman self-assigned this May 8, 2019
@sgowroji sgowroji added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 15, 2023
@sgowroji
Copy link
Member

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

@sgowroji sgowroji closed this as not planned Won't fix, can't repro, duplicate, stale Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) stale Issues or PRs that are stale (no activity for 30 days) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request
Projects
None yet
Development

No branches or pull requests

4 participants