Skip to content

Conversation

@tjgq
Copy link
Contributor

@tjgq tjgq commented Apr 14, 2022

This is necessary to ensure that the generate-xml.sh action associated with
every test, which embeds the test runner wall time, doesn't execute again when
run on a warm disk cache.

This PR makes the following changes:

  1. Add SpawnResult#getStartTime alongside existing #getWallTime and populate
    it in various SpawnRunner implementations.
  2. Populate the ExecutedActionMetadata with the start/wall times for a
    locally executed spawn when storing it in the disk cache.
  3. Populate the SpawnResult with the start/wall times for a spawn
    retrieved from the disk cache.
  4. Add a regression test for the scenario described above.

In order to avoid changes to the remote execution protocol, the start/end
time are stored in the existing ActionResult.execution_metadata fields.
Since there is some mismatch between this field and the metrics available
in a SpawnResult, this might be revisited later.

Fixes #14426.

@tjgq tjgq requested a review from a team as a code owner April 14, 2022 13:58
@tjgq tjgq requested a review from coeuvre April 14, 2022 13:58
@tjgq tjgq force-pushed the test-result-disk-cache branch from 69396f9 to 87a700c Compare April 14, 2022 15:42
@sgowroji
Copy link
Member

Hello @tjgq, Could you please take care of the failure buildkite presubmit checks. Thanks!

@sgowroji sgowroji added the awaiting-user-response Awaiting a response from the author label Apr 19, 2022
@tjgq tjgq force-pushed the test-result-disk-cache branch from 87a700c to f86b174 Compare April 19, 2022 08:52
This is necessary to ensure that the generate-xml.sh action associated with
every test, which embeds the test runner wall time, doesn't execute again when
run on a warm disk cache.

This PR makes the following changes:

1. Add SpawnResult#getStartTime alongside existing #getWallTime and populate
   it in various SpawnRunner implementations.
2. Populate the ExecutedActionMetadata with the start/wall times for a
   locally executed spawn when storing it in the disk cache.
3. Populate the SpawnResult with the start/wall times for a spawn
   retrieved from the disk cache.
4. Add a regression test for the scenario described above.

In order to avoid changes to the remote execution protocol, the start/end
time are stored in the existing ActionResult.execution_metadata fields.
Since there is some mismatch between this field and the metrics available
in a SpawnResult, this might be revisited later.

Fixes bazelbuild#14426.
@tjgq tjgq force-pushed the test-result-disk-cache branch from f86b174 to 6220041 Compare April 19, 2022 10:41
@sgowroji sgowroji added team-Remote-Exec Issues and PRs for the Execution (Remote) team and removed awaiting-user-response Awaiting a response from the author labels Apr 19, 2022
@bazel-io bazel-io closed this in 93029d8 Apr 20, 2022
@brentleyjones
Copy link
Contributor

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Apr 20, 2022
@ckolli5
Copy link

ckolli5 commented Apr 21, 2022

@bazel-io fork 5.2.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Apr 21, 2022
ckolli5 added a commit that referenced this pull request May 9, 2022
…che. (#15439)

This is necessary to ensure that the generate-xml.sh action associated with
every test, which embeds the test runner wall time, doesn't execute again when
run on a warm disk cache.

This PR makes the following changes:

1. Add SpawnResult#getStartTime alongside existing #getWallTime and populate
   it in various SpawnRunner implementations.
2. Populate the ExecutedActionMetadata with the start/wall times for a
   locally executed spawn when storing it in the disk cache.
3. Populate the SpawnResult with the start/wall times for a spawn
   retrieved from the disk cache.
4. Add a regression test for the scenario described above.

In order to avoid changes to the remote execution protocol, the start/end
time are stored in the existing ActionResult.execution_metadata fields.
Since there is some mismatch between this field and the metrics available
in a SpawnResult, this might be revisited later.

Fixes #14426.

Closes #15256.

PiperOrigin-RevId: 443052342

Co-authored-by: Tiago Quelhas <[email protected]>
@tjgq tjgq deleted the test-result-disk-cache branch December 8, 2022 18:07
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.

Test report fails to get a cache hit the second time it runs

6 participants