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

add default backlog to socket.listen() #65654

Closed
neologix mannequin opened this issue May 8, 2014 · 11 comments
Closed

add default backlog to socket.listen() #65654

neologix mannequin opened this issue May 8, 2014 · 11 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@neologix
Copy link
Mannequin

neologix mannequin commented May 8, 2014

BPO 21455
Nosy @pitrou, @vstinner
Files
  • socket_listen.diff
  • socket_listen-1.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2014-05-22.20:45:13.362>
    created_at = <Date 2014-05-08.19:13:27.028>
    labels = ['type-feature', 'library']
    title = 'add default backlog to socket.listen()'
    updated_at = <Date 2014-05-22.20:45:13.361>
    user = 'https://bugs.python.org/neologix'

    bugs.python.org fields:

    activity = <Date 2014-05-22.20:45:13.361>
    actor = 'neologix'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-05-22.20:45:13.362>
    closer = 'neologix'
    components = ['Library (Lib)']
    creation = <Date 2014-05-08.19:13:27.028>
    creator = 'neologix'
    dependencies = []
    files = ['35186', '35188']
    hgrepos = []
    issue_num = 21455
    keywords = ['patch']
    message_count = 11.0
    messages = ['218121', '218127', '218135', '218278', '218279', '218369', '218374', '218401', '218411', '218910', '218919']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'vstinner', 'neologix', 'python-dev', 'sbt']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21455'
    versions = ['Python 3.5']

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented May 8, 2014

    Having to pass an explicit backlog value to listen() is a pain: most people don't know which value to pass, and often end up using a value too small which can lead to connections being rejected.
    For example, if you search throughout the standard library, you'll see many different hard-coded values.

    The patch attached uses a default value of SOMAXCONN for socket.listen(): it should give a good default behavior (that's what golang does for example: in fact they go even further by checking the runtime value from net.core.somaxconn).

    A second step would be to update the codebase to remove explicit backlogs (unless specific case).

    @neologix neologix mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels May 8, 2014
    @pitrou
    Copy link
    Member

    pitrou commented May 8, 2014

    Is there a risk of SOMAXCONN being huge and therefore allocating a large amount of resources?

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented May 8, 2014

    Is there a risk of SOMAXCONN being huge and therefore allocating a large amount of resources?

    On a sensible operating system, no, but better safe than sorry: the
    patch attached caps the value to 128 (a common SOMAXCONN value).
    It should be high enough to avoid connection drops in common
    workloads, and still guard against non-sensical SOMAXCONN values.

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented May 11, 2014

    Any objection to the last version?

    If not, I'll commit it within a few days.

    @pitrou
    Copy link
    Member

    pitrou commented May 11, 2014

    Looks good to me!

    @vstinner
    Copy link
    Member

    vstinner commented May 12, 2014

    Py_MIN(SOMAXCONN, 128)

    On Windows, it's not the best choice to use this hardcoded limit. socket.SOMAXCONN is 2^31-1.

    listen() documentation says that Windows chooses a reasonable backlog value for you if you pass SOMAXCONN:
    http://msdn.microsoft.com/en-us/library/windows/desktop/ms739168%28v=vs.85%29.aspx

    You should maybe use SOMAXCONN by default on Windows, and Py_MIN(SOMAXCONN, 128) by default on other platforms (UNIX).

    @vstinner
    Copy link
    Member

    vstinner commented May 12, 2014

    An article suggests to use max(1024, socket.SOMAXCONN) (to "listen() backlog as large as possible") instead of socket.SOMAXCONN because the OS maximum can be larger than SOMAXCONN (and that it's not possible in Python to get the OS value):
    http://utcc.utoronto.ca/~cks/space/blog/python/AvoidSOMAXCONN?showcomments#comments

    The following article tries to explain why the default limit is 128 on Linux:
    https://derrickpetzold.com/p/somaxconn/

    Article giving the value chosen by Windows when SOMAXCONN is passed: it depends on the Windows version (between 5 and 50, hard limit of 200 on Windows 7):
    http://stackoverflow.com/questions/4709756/listen-maximum-queue-size-per-windows-version

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented May 13, 2014

    > Py_MIN(SOMAXCONN, 128)

    On Windows, it's not the best choice to use this hardcoded limit. socket.SOMAXCONN is 2^31-1.

    I don't see what this would bring: 128 *is* a reasonable limit.

    listen() documentation says that Windows chooses a reasonable backlog value for you if you pass SOMAXCONN:
    http://msdn.microsoft.com/en-us/library/windows/desktop/ms739168%28v=vs.85%29.aspx

    You should maybe use SOMAXCONN by default on Windows, and Py_MIN(SOMAXCONN, 128) by default on other platforms (UNIX).

    Trying to come up with a good heuristic is a lost battle, since it
    depends not only on the OS but also the application. The goal is to
    have a default value that works, is large enough to avoid connection
    drops in common use cases, and not too large to avoid resource
    consumption.

    An article suggests to use max(1024, socket.SOMAXCONN) (to "listen() backlog as large as possible")
    instead of socket.SOMAXCONN because the OS maximum can be larger than SOMAXCONN (and that
    it's not possible in Python to get the OS value):

    In theory we could use net.core.somaxconn sysctl & Co (that's what go
    does), but IMO it's overkill.

    @vstinner
    Copy link
    Member

    vstinner commented May 13, 2014

    Hum ok, thanks for your explanation Charles-François. socket_listen-1.diff looks good to me.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 22, 2014

    New changeset 09371221e59d by Charles-François Natali in branch 'default':
    Issue bpo-21455: Add a default backlog to socket.listen().
    http://hg.python.org/cpython/rev/09371221e59d

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented May 22, 2014

    Committed, thanks!

    @neologix neologix mannequin closed this as completed May 22, 2014
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants