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

Notebook "ulimit -v" does not work #11939

Closed
jdemeyer opened this issue Oct 18, 2011 · 13 comments
Closed

Notebook "ulimit -v" does not work #11939

jdemeyer opened this issue Oct 18, 2011 · 13 comments

Comments

@jdemeyer
Copy link

The ulimit -v argument does not do anything at all (I have not tested other ulimit options), neither locally nor remote. Both commands

notebook(ulimit="-v 1")

and

notebook(server_pool=["jdemeyer@localhost"], ulimit="-v 1")

don't seem to put any limit.

Upstream: Reported upstream. Developers acknowledge bug.

CC: @fchapoton

Component: notebook

Issue created by migration from https://trac.sagemath.org/ticket/11939

@jdemeyer

This comment has been minimized.

@ppurka
Copy link
Member

ppurka commented Oct 18, 2011

comment:2

I looked at the code couple of months ago and what it does is baffling. It parses all the parameters and then recombines them and tries to call ulimit. No idea why it does so.

In my opinion, setting ulimit should be an argument to SAGE_ROOT/sage, i.e., SAGE_ROOT/local/bin/sage-sage. Then ulimit should be set by running ulimit <along with the options passed without parsing them> and then Sage or the Sage notebook should be launched. Since Sage or Sage nb will be running in the same shell, it will run with the ulimits that were set.

@jdemeyer
Copy link
Author

comment:3

Replying to @ppurka:

Since Sage or Sage nb will be running in the same shell, it will run with the ulimits that were set.

This is not true for remote notebook workers, so we need something more clever.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Oct 18, 2011

comment:4

It seems at least your example (-v 1) yields a zero limit, since the value is first divided by 1000.0, then gets converted back to an int, and afterwards gets multiplied by 1000 again.

@ppurka
Copy link
Member

ppurka commented Oct 18, 2011

comment:5

Even with all the conversions, I am pretty sure ulimit does not work. That's what brought down our servers on the first day of classes. Everyone had -v set to 2G and yet the Sage process of one person was allowed to run up 20G+ of memory, eating up all the available ram and swap.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Oct 18, 2011

comment:6

You may try this (haven't tested the notebook at all):

diff --git a/sagenb/interfaces/expect.py b/sagenb/interfaces/expect.py
--- a/sagenb/interfaces/expect.py
+++ b/sagenb/interfaces/expect.py
@@ -47,7 +47,7 @@
         if process_limits:
             u = ''
             if process_limits.max_vmem is not None:
-                u += ' -v %s'%(int(process_limits.max_vmem)*1000)
+                u += ' -v %s'%(int(process_limits.max_vmem*1000))
             if process_limits.max_cputime is not None:
                 u += ' -t %s'%(int(process_limits.max_cputime))
             if process_limits.max_processes is not None:

@ppurka
Copy link
Member

ppurka commented Oct 18, 2011

comment:7

Oh, also you can check whether ulimit is set by running this from a worksheet:

import os
os.system("ulimit -a")

@nexttime
Copy link
Mannequin

nexttime mannequin commented Oct 18, 2011

comment:8

**Hahahahaha!!!1! **

    def command(self):
        return self._python
        # TODO: The following simply doesn't work -- this is not a valid way to run
        # ulimited.  Also we should check if ulimit is available before even
        # doing this.   
        return '&&'.join([x for x in [self._ulimit, self._python] if x])

(This is WorksheetProcess_ExpectImplementation.command().)

@jdemeyer
Copy link
Author

jdemeyer commented Nov 3, 2011

Milestone sage-4.7.3 deleted

@jdemeyer jdemeyer removed this from the sage-4.8 milestone Nov 3, 2011
@novoselt
Copy link
Member

comment:10

In my experience the only working way to set limits is to edit the sage script to call ulimit if the username is "Sage worker". It is quite annoying to remember to do it between upgrades and in any case this is not what the documentation tells us to do...

@novoselt novoselt added this to the sage-5.7 milestone Jan 21, 2013
@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@kcrisman
Copy link
Member

Upstream: Reported upstream. Developers acknowledge bug.

@kcrisman
Copy link
Member

comment:15

See sagemath/sagenb#235

@mkoeppe
Copy link
Member

mkoeppe commented Aug 18, 2020

comment:16

Proposing to close all sagenb tickets as outdated, so that all remaining open tickets in the notebook component are about the Jupyter notebook.

@mkoeppe mkoeppe removed this from the sage-6.4 milestone Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants