-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Docker: memory_hard_limit conflict with MemorySwap #8153
Comments
Thanks for reporting this, @njenwei ! When I try your reproduction example on my dev machine it works fine. I'm thinking this may be an incompatibility with older kernels - for reference 3.10 came out in 2013.
|
@shoenig Thanks for your quick response! Unfortunately, I am able to recreate this on the Mac OS as well:
See output when running in dev mode:
Are you able to check your memory configuration for the docker container that nomad is launching on your PC? Something like |
I think your original observation is correct, @njenwei . After reading through the memory swap docs again with a magnifying glass, I believe we are currently setting the incorrect value for The assignment hostConfig.MemorySwap = task.Resources.LinuxResources.MemoryLimitBytes does not take into account the case when - hostConfig.MemorySwap = task.Resources.LinuxResources.MemoryLimitBytes // MemorySwap is memory + swap.
+ // --memory-swap should always be same as --memory (hard limit) so as to disable swap.
+ //
+ // More information in https://docs.docker.com/config/containers/resource_constraints/#--memory-swap-details
+ hostConfig.MemorySwap = memory Testing this out locally produces identical results:
Interesting that the @njenwei do you mind testing out that patch and confirming whether it solves the issue? If it checks out, would you also like to open a PR? If not I can take care of it ~ |
Fixes an incorrect value being assigned to MemorySwap when `memory_hard_limit` flag is being used. Issue raised in hashicorp#8153
With the change, I am now getting expected values when inspecting the container on mac:
Unfortunately I won't have the opportunity to test this out on a Linux box until Monday. I'm not sure why your MemorySwap is being set to -1, it might be worth looking into what the |
I can also confirm that the change works on my original OS:
@shoenig are you happy to merge the changes? |
Out of curiosity I tested with/without the fix on Ubuntu 20.04, 16.04, CentOS 7 and RHEL 8 using ec2 AMIs. 20.04
16.04
CentOS 7
RHEL 8
@njenwei I do believe this fix makes Nomad's use of the feature correct with respect to Docker's documentation. Looking at the differences above, I realize this may just be the result of a difference in default swap settings between OS's. 20.04
RHEL 8
|
I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues. |
Nomad version
Operating system and Environment details
Issue
Unable to create containers when memory_hard_limit > memory.
I am not familar with Go, but could this be due to an incorrect
hostConfig.MemorySwap
value being set when using the optionmemory_hard_limit
as shown innomad/drivers/docker/driver.go
Line 841 in 9ccec0a
I feel like it should be
instead of
Reproduction steps
Run job file
Job file (if appropriate)
This was also raise in the original PR: #8087 (comment)
The text was updated successfully, but these errors were encountered: