Fix workingset memory calculation. #766
Conversation
|
@Random-Liu: GitHub didn't allow me to request PR reviews from the following users: dashpole. Note that only containerd members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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. |
Random-Liu
force-pushed
the
fix-workingset-memory
branch
2 times, most recently
from
f8a1b75
to
7734ff6
Compare
pkg/server/container_stats_list.go
Outdated
| @@ -107,10 +107,12 @@ func (c *criService) getContainerMetrics( | |||
| UsageCoreNanoSeconds: &runtime.UInt64Value{Value: metrics.CPU.Usage.Total}, | |||
| } | |||
| } | |||
| if metrics.Memory != nil && metrics.Memory.Usage != nil { | |||
| if metrics.Memory != nil { | |||
There was a problem hiding this comment.
since we dont check metrics.Memory.Usage != nil anymore, missing memory metrics result in a adding a value with WorkingSet = 0, instead of cs.Memory = nil. I don't know the codebase, so I don't know if this matters.
There was a problem hiding this comment.
Hm, good catch. I'll keep the behavior unchanged.
Random-Liu
force-pushed
the
fix-workingset-memory
branch
from
7734ff6
to
3f9f2c4
Compare
pkg/server/container_stats_list.go
Outdated
| if memory.Usage == nil { | ||
| return 0 | ||
| } | ||
| workingSet := memory.Usage.Usage |
pkg/server/container_stats_list.go
Outdated
| return 0 | ||
| } | ||
| workingSet := memory.Usage.Usage | ||
| if memory.TotalInactiveFile < workingSet { |
There was a problem hiding this comment.
should compare with memory.Usage.Usage here
pkg/server/container_stats_list.go
Outdated
| workingSet := memory.Usage.Usage | ||
| if memory.TotalInactiveFile < workingSet { | ||
| workingSet -= memory.TotalInactiveFile | ||
| } else { |
pkg/server/container_stats_list.go
Outdated
| } | ||
| workingSet := memory.Usage.Usage | ||
| if memory.TotalInactiveFile < workingSet { | ||
| workingSet -= memory.TotalInactiveFile |
There was a problem hiding this comment.
workingSet = memory.Usage.Usage - memory.TotalInactiveFile..
Signed-off-by: Lantao Liu <lantaol@google.com>
Random-Liu
force-pushed
the
fix-workingset-memory
branch
from
3f9f2c4
to
5d29598
Compare
|
@mikebrow Done. Cleaner. |
Cherrypick #766 into release/1.0 branch.
Fix container workingset memory calculation.
In kubernetes and cadvisor, workingset is defined as "usage - total_inactive_file".
See https://github.com/google/cadvisor/blob/08f0c2397cbca790a4db0f1212cb592cc88f6e26/container/libcontainer/handler.go#L520.
/cc @dashpole @abhi
Signed-off-by: Lantao Liu lantaol@google.com