Skip to content

Cumulative stats can't decrease#10652

Merged
dmcgowan merged 1 commit intocontainerd:mainfrom
jsturtevant:fix-nano-calculation
Aug 29, 2024
Merged

Cumulative stats can't decrease#10652
dmcgowan merged 1 commit intocontainerd:mainfrom
jsturtevant:fix-nano-calculation

Conversation

@jsturtevant
Copy link
Copy Markdown
Contributor

@jsturtevant jsturtevant commented Aug 28, 2024 •
edited
Loading

During removal of the container a stat value might be reported as zero; in this case the calculation could end up with an extremely large number. If the cumulative stat decreases report zero.

fixes kubernetes/kubernetes#114057

@dosubot dosubot Bot added area/cri Container Runtime Interface (CRI) kind/bug labels Aug 28, 2024
@jsturtevant
Copy link
Copy Markdown
Contributor Author

jsturtevant commented Aug 28, 2024

follows https://github.com/google/cadvisor/blob/c15f44e578c77800b1b82a7bbb67614364f4aedc/info/v2/conversion.go#L247-L249

@jsturtevant jsturtevant mentioned this pull request Aug 28, 2024
Closed
@jsturtevant
Cumulative stats can't decrease
f6677a4 
During removal of the container a stat value might be reported as zero; in this case the caluclation could end up with an extremely large number.  If the cumulative stat decreases report zero.

Signed-off-by: James Sturtevant <[email protected]>
@jsturtevant jsturtevant force-pushed the fix-nano-calculation branch from b5cc531 to f6677a4 Compare August 28, 2024 23:23
cpuguy83
cpuguy83 approved these changes Aug 28, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@jsturtevant
Copy link
Copy Markdown
Contributor Author

jsturtevant commented Aug 29, 2024

/test pull-containerd-node-e2e
looks like unrelated flake

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Aug 29, 2024

Backport 1.6 and 1.7?

@cpuguy83 cpuguy83 added cherry-pick/1.6.x cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Aug 29, 2024
@cpuguy83
Copy link
Copy Markdown
Member

cpuguy83 commented Aug 29, 2024

Yep, I think so. I added those labels.

@jsturtevant
Copy link
Copy Markdown
Contributor Author

jsturtevant commented Aug 29, 2024

yes, we are seeing it in 1.6

@dmcgowan dmcgowan added this pull request to the merge queue Aug 29, 2024
Merged via the queue into containerd:main with commit dacde84 Aug 29, 2024
@jsturtevant jsturtevant mentioned this pull request Sep 3, 2024
Merged
@samuelkarp
Copy link
Copy Markdown
Member

samuelkarp commented Sep 3, 2024

/cherrypick release/1.6

@k8s-infra-cherrypick-robot
Copy link
Copy Markdown

k8s-infra-cherrypick-robot commented Sep 3, 2024

@samuelkarp: #10652 failed to apply on top of branch "release/1.6":

Applying: Cumulative stats can't decrease
Using index info to reconstruct a base tree...
A	internal/cri/server/container_stats_list.go
A	internal/cri/server/container_stats_list_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/cri/server/container_stats_list_linux_test.go
CONFLICT (content): Merge conflict in pkg/cri/server/container_stats_list_linux_test.go
CONFLICT (modify/delete): internal/cri/server/container_stats_list.go deleted in HEAD and modified in Cumulative stats can't decrease. Version Cumulative stats can't decrease of internal/cri/server/container_stats_list.go left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Cumulative stats can't decrease
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Details

In response to this:

/cherrypick release/1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@jsturtevant
Copy link
Copy Markdown
Contributor Author

jsturtevant commented Sep 3, 2024 •
edited
Loading

didn't know there was a bot, since this is in cri server it required some manual changes for SBServer changes in 1.6/1.7: #10671

This was referenced Sep 3, 2024
Merged
Closed
@austinvazquez austinvazquez added cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch and removed cherry-pick/1.6.x cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmcgowan dmcgowan dmcgowan approved these changes

@cpuguy83 cpuguy83 cpuguy83 approved these changes

Assignees

No one assigned

Labels

area/cri Container Runtime Interface (CRI) cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch kind/bug size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Impossibly high cpu usage value from kubelet embedded cadvisor (k8s 1.23.12)

7 participants

@jsturtevant @dmcgowan @cpuguy83 @samuelkarp @k8s-infra-cherrypick-robot @k8s-ci-robot @austinvazquez