[issue21455] add default backlog to socket.listen()
Roundup Robot added the comment: New changeset 09371221e59d by Charles-François Natali in branch 'default': Issue #21455: Add a default backlog to socket.listen(). http://hg.python.org/cpython/rev/09371221e59d -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21455] add default backlog to socket.listen()
Charles-François Natali added the comment: Committed, thanks! -- resolution: - fixed stage: patch review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21455] add default backlog to socket.listen()
STINNER Victor added the comment: Hum ok, thanks for your explanation Charles-François. socket_listen-1.diff looks good to me. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21455] add default backlog to socket.listen()
STINNER Victor added the comment: 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). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21455] add default backlog to socket.listen()
STINNER Victor added the comment: 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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21455] add default backlog to socket.listen()
Charles-François Natali added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21455] add default backlog to socket.listen()
Charles-François Natali added the comment: Any objection to the last version? If not, I'll commit it within a few days. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21455] add default backlog to socket.listen()
Antoine Pitrou added the comment: Looks good to me! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21455] add default backlog to socket.listen()
New submission from Charles-François Natali: 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). -- components: Library (Lib) files: socket_listen.diff keywords: patch messages: 218121 nosy: haypo, neologix, pitrou, sbt priority: normal severity: normal stage: patch review status: open title: add default backlog to socket.listen() type: enhancement versions: Python 3.5 Added file: http://bugs.python.org/file35186/socket_listen.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21455] add default backlog to socket.listen()
Antoine Pitrou added the comment: Is there a risk of SOMAXCONN being huge and therefore allocating a large amount of resources? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue21455] add default backlog to socket.listen()
Charles-François Natali added the comment: 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. -- Added file: http://bugs.python.org/file35188/socket_listen-1.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue21455 ___diff -r 17689e43839a Doc/library/socket.rst --- a/Doc/library/socket.rstThu May 08 23:08:51 2014 +0100 +++ b/Doc/library/socket.rstThu May 08 23:52:22 2014 +0100 @@ -906,12 +906,15 @@ On other platforms, the generic :func:`fcntl.fcntl` and :func:`fcntl.ioctl` functions may be used; they accept a socket object as their first argument. -.. method:: socket.listen(backlog) +.. method:: socket.listen([backlog]) - Listen for connections made to the socket. The *backlog* argument specifies the - maximum number of queued connections and should be at least 0; the maximum value - is system-dependent (usually 5), the minimum value is forced to 0. + Enable a server to accept connections. If *backlog* is specified, it must + be at least 0 (if it is lower, it is set to 0); it specifies the number of + unaccepted connections that the system will allow before refusing new + connections. If not specified, a default reasonable value is chosen. + .. versionchanged:: 3.5 + The *backlog* parameter is now optional. .. method:: socket.makefile(mode='r', buffering=None, *, encoding=None, \ errors=None, newline=None) diff -r 17689e43839a Lib/test/test_socket.py --- a/Lib/test/test_socket.py Thu May 08 23:08:51 2014 +0100 +++ b/Lib/test/test_socket.py Thu May 08 23:52:22 2014 +0100 @@ -1344,10 +1344,13 @@ def test_listen_backlog(self): for backlog in 0, -1: -srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM) +with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as srv: +srv.bind((HOST, 0)) +srv.listen(backlog) + +with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as srv: srv.bind((HOST, 0)) -srv.listen(backlog) -srv.close() +srv.listen() @support.cpython_only def test_listen_backlog_overflow(self): diff -r 17689e43839a Modules/socketmodule.c --- a/Modules/socketmodule.cThu May 08 23:08:51 2014 +0100 +++ b/Modules/socketmodule.cThu May 08 23:52:22 2014 +0100 @@ -121,7 +121,7 @@ getsockname() -- return local address\n\ getsockopt(level, optname[, buflen]) -- get socket options\n\ gettimeout() -- return timeout or None\n\ -listen(n) -- start listening for incoming connections\n\ +listen([n]) -- start listening for incoming connections\n\ recv(buflen[, flags]) -- receive data\n\ recv_into(buffer[, nbytes[, flags]]) -- receive data (into a buffer)\n\ recvfrom(buflen[, flags]) -- receive data and sender\'s address\n\ @@ -2534,14 +2534,16 @@ /* s.listen(n) method */ static PyObject * -sock_listen(PySocketSockObject *s, PyObject *arg) +sock_listen(PySocketSockObject *s, PyObject *args) { -int backlog; +/* We try to choose a default backlog high enough to avoid connection drops + * for common workloads, yet not too high to limit resource usage. */ +int backlog = Py_MIN(SOMAXCONN, 128); int res; -backlog = _PyLong_AsInt(arg); -if (backlog == -1 PyErr_Occurred()) +if (!PyArg_ParseTuple(args, |i:listen, backlog)) return NULL; + Py_BEGIN_ALLOW_THREADS /* To avoid problems on systems that don't allow a negative backlog * (which doesn't make sense anyway) we force a minimum value of 0. */ @@ -2556,12 +2558,12 @@ } PyDoc_STRVAR(listen_doc, -listen(backlog)\n\ +listen([backlog])\n\ \n\ -Enable a server to accept connections. The backlog argument must be at\n\ -least 0 (if it is lower, it is set to 0); it specifies the number of\n\ +Enable a server to accept connections. If backlog is specified, it must be\n\ +at least 0 (if it is lower, it is set to 0); it specifies the number of\n\ unaccepted connections that the system will allow before refusing new\n\ -connections.); +connections. If not specified, a default reasonable value is chosen.); /* @@ -3795,7 +3797,7 @@ {share, (PyCFunction)sock_share, METH_VARARGS, sock_share_doc}, #endif -{listen,(PyCFunction)sock_listen, METH_O, +{listen,(PyCFunction)sock_listen, METH_VARARGS, listen_doc}, {recv, (PyCFunction)sock_recv, METH_VARARGS, recv_doc}, ___ Python-bugs-list mailing list