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

Limit virtual memory for linters #108

Merged
merged 2 commits into from
Aug 7, 2017
Merged

Limit virtual memory for linters #108

merged 2 commits into from
Aug 7, 2017

Conversation

bradleyfalzon
Copy link
Owner

Previously any tool running could consume all available RAM
and swap causing availability issues if the offending process
isn't terminated quickly.

This change adds a configuration parameter to limit the amount
of RAM a process can use. If it exceeds this limit, the process
is terminated, but other processes can continue.

Although we have a Filesystem analyser, which can only limit
memory usage via ulimit, the Docker analyser also uses this
mechanism because Docker's own memory limit will terminate the
container, not just the offending process. I.e. the limit cannot
be applied to our exec instance, only when we've created the
container.

We're also required to run the ulimit command each time, because
each tool is executed with a new shell, which won't inherit the
limits put in place by previous commands.

Fixes #106.

This keeps it consistent with Docker analyser.
Previously any tool running could consume all available RAM
and swap causing availability issues if the offending process
isn't terminated quickly.

This change adds a configuration parameter to limit the amount
of RAM a process can use. If it exceeds this limit, the process
is terminated, but other processes can continue.

Although we have a Filesystem analyser, which can only limit
memory usage via ulimit, the Docker analyser also uses this
mechanism because Docker's own memory limit will terminate the
container, not just the offending process. I.e. the limit cannot
be applied to our exec instance, only when we've created the
container.

We're also required to run the ulimit command each time, because
each tool is executed with a new shell, which won't inherit the
limits put in place by previous commands.

Fixes #106.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 59.974% when pulling db57ab5 on memlimit into 53fd41e on master.

@bradleyfalzon
Copy link
Owner Author

Merging this quickly as it's holding up another PR.

@bradleyfalzon bradleyfalzon merged commit 04a08da into master Aug 7, 2017
@bradleyfalzon bradleyfalzon deleted the memlimit branch August 7, 2017 03:59
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.

None yet

2 participants