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

[MRG] Update base image used for memory limit checks #677

Merged
merged 6 commits into from
May 8, 2019

Conversation

betatim
Copy link
Member

@betatim betatim commented May 7, 2019

Locally this still fails :-/

@betatim
Copy link
Member Author

betatim commented May 7, 2019

It seems like (locally) the limit doesn't actually get applied/used :-/ I'll need a bit of time to investigate what is going on.

@Xarthisius
Copy link
Contributor

Locally this still fails :-/

@betatim I vaguely remember that I had problem imposing mem limits for docker using python API. It accepted the size notation but it wasn't setting the limit properly. I'd try to use number of bytes explicitly.

@betatim
Copy link
Member Author

betatim commented May 7, 2019

When we finally actually call the docker client the size is an integer without a postfix :-/

@betatim
Copy link
Member Author

betatim commented May 8, 2019

Mystery solved. The docker-py documentation says that a value of -1 will disable swap, which we were using. However https://docs.docker.com/engine/reference/commandline/build/ specifies that -1 means "unlimited swap". In the docker-py code I can't find anywhere where they do processing of the values we passed in.

Updated our code to reflect this new knowledge. Though I am confused why all tests have passed on travis (for 5adc4b2) when they fail locally. Speculation: there is no swap for the whole machine on travis?

@betatim betatim force-pushed the fix-memlimit-image branch from 8588fe2 to 032baf6 Compare May 8, 2019 12:23
@betatim betatim changed the title [WIP] Update base image used for memory limit checks [MRG] Update base image used for memory limit checks May 8, 2019
@yuvipanda
Copy link
Collaborator

I dug around a bit more, and found docker/docs#4335. Is there a reason we're setting memory swap to be one byte more, instead of exactly the same?

@betatim
Copy link
Member Author

betatim commented May 8, 2019

When I had it set to 1 (a guess for "if we can't disable then let's use 1byte") I got an error the said "swap has to be bigger than memory limit", I didn't check if they actually meant "equal or larger". Checking now.

@betatim
Copy link
Member Author

betatim commented May 8, 2019

Looks like setting it to the same amount works! That is a nicer solution :D Thanks for asking.

@yuvipanda
Copy link
Collaborator

@betatim awesome! I changed the comment around it a little, otherwise it LGTM.

@yuvipanda yuvipanda merged commit 2086b7a into jupyterhub:master May 8, 2019
@yuvipanda
Copy link
Collaborator

Merged with @betatim's blessing. <3

@betatim betatim deleted the fix-memlimit-image branch May 8, 2019 20:03
markmo pushed a commit to markmo/repo2docker that referenced this pull request Jan 22, 2021
[MRG] Update base image used for memory limit checks
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.

3 participants