Skip to content

fix: fix up memory usage for cgroup v2#12703

Closed
ttys3 wants to merge 1 commit into
podman-container-tools:mainfrom
ttys3:cgroupv2-memory-usage-fixup
Closed

fix: fix up memory usage for cgroup v2#12703
ttys3 wants to merge 1 commit into
podman-container-tools:mainfrom
ttys3:cgroupv2-memory-usage-fixup

Conversation

@ttys3

@ttys3 ttys3 commented Dec 25, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

2022-01-15 update:

this PR has been included in #12786 , so this one could be closed

https://github.com/containers/podman/pull/12786/files#diff-e4a5be3ec14dc0340e98adece8398a26007a4e9aaed9f12a285e2c0690262a01L44

as described in #12702

this PR depends on upstream PR containers/common#870

related file:

 M vendor/github.com/containers/common/pkg/cgroups/cgroups.go
 M vendor/github.com/containers/common/pkg/cgroups/memory.go

@ttys3

ttys3 commented Dec 25, 2021

Copy link
Copy Markdown
Contributor Author

/assign @umohnani8

@ttys3 ttys3 mentioned this pull request Dec 25, 2021
Closed
@ttys3 ttys3 force-pushed the cgroupv2-memory-usage-fixup branch from 33ca8cc to 3958cc2 Compare December 25, 2021 16:26
@ttys3 ttys3 mentioned this pull request Dec 26, 2021
Merged
@rhatdan rhatdan changed the title fix: fix up memory usage for cgroup v2 [wip] fix: fix up memory usage for cgroup v2 Dec 26, 2021
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 26, 2021
@rhatdan

rhatdan commented Dec 26, 2021

Copy link
Copy Markdown
Contributor

You want to do something like

go mod edit --replace github.com/containers/common=github.com/ttys3/common@ff0756f34c46b413ea0f179942e0b517cc873fd5

make vendor-in-container

And then git push --force

@ttys3

ttys3 commented Dec 26, 2021

Copy link
Copy Markdown
Contributor Author

@rhatdan thank you. I got it.
Is it better to wait the official upstream got merged ?

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 26, 2021
@ttys3 ttys3 force-pushed the cgroupv2-memory-usage-fixup branch from 5ba331a to 449c649 Compare December 26, 2021 14:42
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 26, 2021
@ttys3 ttys3 force-pushed the cgroupv2-memory-usage-fixup branch 3 times, most recently from 0be4390 to b1af6ae Compare December 26, 2021 15:25
@rhatdan rhatdan changed the title [wip] fix: fix up memory usage for cgroup v2 fix: fix up memory usage for cgroup v2 Dec 26, 2021
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 26, 2021
@rhatdan

rhatdan commented Dec 26, 2021

Copy link
Copy Markdown
Contributor

Now that is is merged, yes, it is better to wait. :^)

LGTM
/approve

@openshift-ci

openshift-ci Bot commented Dec 26, 2021

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, ttys3

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 26, 2021
@ttys3

ttys3 commented Dec 26, 2021

Copy link
Copy Markdown
Contributor Author

seems not related to this PR CI tests failed: https://cirrus-ci.com/task/5146326085140480?logs=main#L163

[+0020s] not ok 156 [10-images] POST images/prune?filters={"dangling":["true"]} [-d {}] : status
[+0020s] #  expected: 200
[+0020s] #    actual: 500
[+0020s] #  response: {"cause":"specifying \"dangling\" filter more then once is not supported","message":"specifying \"dangling\" filter more then once is not supported","response":500}
[+0020s] ok 157 [10-images] GET images/json?filters={"dangling":["true"]} : status=200
[+0020s] not ok 158 [10-images] GET images/json?filters={"dangling":["true"]} : length
[+0020s] #  expected: 0

@rhatdan

rhatdan commented Dec 26, 2021

Copy link
Copy Markdown
Contributor

#12572 has the changes needed to get containers/common revendored.

@ttys3

ttys3 commented Dec 27, 2021

Copy link
Copy Markdown
Contributor Author

OK, I re-vendor like #12572

use github.com/containers/common v0.46.1-0.20211226134707-c7fd073cc020

@ttys3 ttys3 force-pushed the cgroupv2-memory-usage-fixup branch from b1af6ae to df647f3 Compare December 27, 2021 18:05
@ttys3
fix: fix up memory usage for cgroup v2
ff9a3f5 
chore: update to github.com/containers/common v0.46.1-0.20211226134707-c7fd073cc020

containers/common@c7fd073

Signed-off-by: ttyS3 <ttys3.rust@gmail.com>
@ttys3 ttys3 force-pushed the cgroupv2-memory-usage-fixup branch from df647f3 to ff9a3f5 Compare December 27, 2021 18:16
@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 11, 2022
@openshift-ci

openshift-ci Bot commented Jan 11, 2022

Copy link
Copy Markdown
Contributor

@ttys3: PR needs rebase.

Details

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/test-infra repository.

@ttys3

ttys3 commented Jan 15, 2022

Copy link
Copy Markdown
Contributor Author

@rhatdan
this PR has been included in #12786 , so this one could be closed

https://github.com/containers/podman/pull/12786/files#diff-e4a5be3ec14dc0340e98adece8398a26007a4e9aaed9f12a285e2c0690262a01L44

@ttys3 ttys3 closed this Jan 15, 2022
@github-actions github-actions Bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@umohnani8 umohnani8

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ttys3 @rhatdan @umohnani8