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

[Telemetry docker] add memory and memory swap limits #7062

Merged
merged 2 commits into from
May 11, 2021

Conversation

pra-moh
Copy link
Contributor

@pra-moh pra-moh commented Mar 15, 2021

Why I did it

Fix https://github.com/Azure/sonic-telemetry/issues/71

How I did it

Added memory limit for telemetry docker.
Historical docker memory usage shows telemetry docker consuming 150-200MB memory. Adding some extra buffer.

How to verify it

Build and install image with change:
image

docker memory for telemetry changes to assigned value:
image

Which release branch to backport (provide reason below if selected)

  • 201811
  • [- ] 201911
  • [- ] 202006
  • [- ] 202012

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

@rlhui
Copy link
Contributor

rlhui commented Mar 16, 2021

@pra-moh - thanks, please add testing done.

@rlhui rlhui requested a review from abdosi March 16, 2021 02:57
@lguohan
Copy link
Collaborator

lguohan commented Mar 16, 2021

what is memory-swap?

@pra-moh
Copy link
Contributor Author

pra-moh commented Mar 16, 2021

what is memory-swap?

what is memory-swap?
Adding this argument to avoid container using any extra memory. Using just --memory argument can make docker start memory swap from disk based on some threads. This will make sure it will never use more than 300MB

Based on docker documentation:
PREVENT A CONTAINER FROM USING SWAP
If --memory and --memory-swap are set to the same value, this prevents containers from using any swap. This is because --memory-swap is the amount of combined memory and swap that can be used, while --memory is only the amount of physical memory that can be used.

https://docs.docker.com/config/containers/resource_constraints/#prevent-a-container-from-using-swap

@lguohan
Copy link
Collaborator

lguohan commented Mar 17, 2021

please describe what kind of testing have you done?

besides, we do not use swap at all on switches. it should be set to zero.

@lguohan
Copy link
Collaborator

lguohan commented Apr 14, 2021

can you update the description?

@lguohan
Copy link
Collaborator

lguohan commented Apr 22, 2021

can you update the description to say how 500m is derived?

@rlhui
Copy link
Contributor

rlhui commented May 6, 2021

Let's revise the memory cap based on latest data. Thanks.

@lguohan
Copy link
Collaborator

lguohan commented May 12, 2021

bad pr, oom triggered system reboot.

@hui-ma
Copy link
Contributor

hui-ma commented May 12, 2021

let us revert it. heard about the issue after s/o.

@zhenggen-xu
Copy link
Collaborator

zhenggen-xu commented May 12, 2021

We need "--oom-kill-disable" option when we limit the memory so no kernel panic.

On our testing we found that as we move to Memory threshold CPU utilization start going over 100%+ by telemetry and also docker start getting very slow/unresponsive. High CPU will impact overall system.

Plan is to have monit itself check for telemetry docker memory threshold and if exceed this
We will alert and restart telemetry docker.

carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
#### Why I did it
Fix https://github.com/Azure/sonic-telemetry/issues/71

#### How I did it
Added memory limit for telemetry docker.
Historical docker memory usage shows telemetry docker consuming 150-200MB memory. Adding some extra buffer.
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants