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

Inaccurate "remote cache time saved" total #15080

Closed
stuhood opened this issue Apr 8, 2022 · 1 comment · Fixed by #15088
Closed

Inaccurate "remote cache time saved" total #15080

stuhood opened this issue Apr 8, 2022 · 1 comment · Fixed by #15088
Assignees
Labels
Milestone

Comments

@stuhood
Copy link
Member

stuhood commented Apr 8, 2022

Recent builds of the 2.11.x branch have inaccurate "remote cache time saved" totals in Pants' recorded metrics, although it looks like the histogram is accurate.

@stuhood stuhood added the bug label Apr 8, 2022
@stuhood stuhood added this to the 2.11.x milestone Apr 8, 2022
@stuhood stuhood self-assigned this Apr 8, 2022
@stuhood
Copy link
Member Author

stuhood commented Apr 8, 2022

Unfortunately I don't repro this locally with --stats-log. Will continue investigating.

stuhood added a commit that referenced this issue Apr 11, 2022
…e children (#15088)

#15080 was caused by two factors:
1. #14541 made counters global via a new API, and attached them (as deprecated) to "the root workunit" (with "has no parent id" as the heuristic that something was the root workunit).
2. #14856 moved to calculating the parent(s) of a node based on a running graph of workunits (to allow #14680 to eventually add multiple parents), which meant that when nodes completed out of order, we might not have any parents for them.

Together: this meant that when workunits completed asynchronously, we might not have parents for them, and because of the deprecation, we would attach the counters to multiple workunits. A consumer which was aggregating the counters would end up with an inaccurate total.

Fixes #15080.
stuhood added a commit to stuhood/pants that referenced this issue Apr 11, 2022
…e children (pantsbuild#15088)

1. pantsbuild#14541 made counters global via a new API, and attached them (as deprecated) to "the root workunit" (with "has no parent id" as the heuristic that something was the root workunit).
2. pantsbuild#14856 moved to calculating the parent(s) of a node based on a running graph of workunits (to allow pantsbuild#14680 to eventually add multiple parents), which meant that when nodes completed out of order, we might not have any parents for them.

Together: this meant that when workunits completed asynchronously, we might not have parents for them, and because of the deprecation, we would attach the counters to multiple workunits. A consumer which was aggregating the counters would end up with an inaccurate total.

Fixes pantsbuild#15080.
stuhood added a commit that referenced this issue Apr 11, 2022
…e children (cherrypick of #15088) (#15103)

#15080 was caused by two factors:

1. #14541 made counters global via a new API, and attached them (as deprecated) to "the root workunit" (with "has no parent id" as the heuristic that something was the root workunit).
2. #14856 moved to calculating the parent(s) of a node based on a running graph of workunits (to allow #14680 to eventually add multiple parents), which meant that when nodes completed out of order, we might not have any parents for them.

Together: this meant that when workunits completed asynchronously, we might not have parents for them, and because of the deprecation, we would attach the counters to multiple workunits. A consumer which was aggregating the counters would end up with an inaccurate total.

Fixes #15080.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant