Thread: PQConnectPoll, connect(2), EWOULDBLOCK and somaxconn
Hi, When postgres on linux receives connection on a high rate client connections sometimes error out with: could not send data to server: Transport endpoint is not connected could not send startup packet: Transport endpoint is not connected To reproduce start something like on a server with sufficiently high max_connections: pgbench -h /tmp -p 5440 -T 10 -c 400 -j 400 -n -f /tmp/simplequery.sql Now that's strange since that error should happen at connect(2) time, not when sending the startup packet. Some investigation led me to fe-secure.c's PQConnectPoll: if (connect(conn->sock, addr_cur->ai_addr, addr_cur->ai_addrlen) < 0) { if (SOCK_ERRNO == EINPROGRESS || SOCK_ERRNO == EWOULDBLOCK || SOCK_ERRNO == EINTR || SOCK_ERRNO == 0) { /* * This is fine - we're in non-blocking mode, and * the connection is in progress. Tell caller to * wait for write-ready on socket. */ conn->status = CONNECTION_STARTED; return PGRES_POLLING_WRITING; } /* otherwise, trouble */ } So, we're accepting EWOULDBLOCK as a valid return value for connect(2). Which it isn't. EAGAIN in contrast is on some BSDs and on linux. Unfortunately POSIX allows those two to share the same value... My manpage tells me: EAGAIN No more free local ports or insufficient entries in the routing cache. For AF_INET see the description of /proc/sys/net/ipv4/ip_local_port_range ip(7) for information on how to increase the number of local ports. So, the problem is that we took a failed connection as having been initially successfull but in progress. Not accepting EWOULDBLOCK in the above if() results in: could not connect to server: Resource temporarily unavailable Is the server running locally and accepting connections on Unix domain socket "/tmp/.s.PGSQL.5440"? which makes more sense. Trivial patch attached. Now, the question is why we cannot complete connections on unix sockets? Some code reading reading shows net/unix/af_unix.c:unix_stream_connect() shows: if (unix_recvq_full(other)) { err = -EAGAIN; if (!timeo) goto out_unlock; So, if we're in nonblocking mode - which we are - and the receive queue is full we return EAGAIN. The receive queue for unix sockets is defined as static inline int unix_recvq_full(struct sock const *sk) { return skb_queue_len(&sk->sk_receive_queue) > sk->sk_max_ack_backlog; } Where sk_max_ack_backlog is whatever has been passed to the listen(backlog) on the listening side. Question: But postgres does listen(fd, MaxBackends * 2), how can that be a problem? Answer: If the backlog argument is greater than the value in /proc/sys/net/core/somaxconn, then it is silently truncated to that value; the default value in this file is 128. In kernels before 2.4.25, this limit was a hard coded value, SOMAXCONN, with the value 128. Setting somaxconn to something higher indeed makes the problem go away. I'd guess that pretty much the same holds true for tcp connections, although I didn't verify that which would explain some previous reports on the lists. TLDR: Increase /proc/sys/net/core/somaxconn Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Attachment
Hi, On 2013-06-17 16:16:22 +0200, Andres Freund wrote: > When postgres on linux receives connection on a high rate client > connections sometimes error out with: > could not send data to server: Transport endpoint is not connected > could not send startup packet: Transport endpoint is not connected > > To reproduce start something like on a server with sufficiently high > max_connections: > pgbench -h /tmp -p 5440 -T 10 -c 400 -j 400 -n -f /tmp/simplequery.sql > > Now that's strange since that error should happen at connect(2) time, > not when sending the startup packet. Some investigation led me to > fe-secure.c's PQConnectPoll: > So, we're accepting EWOULDBLOCK as a valid return value for > connect(2). Which it isn't. EAGAIN in contrast is on some BSDs and on > linux. Unfortunately POSIX allows those two to share the same value... > > My manpage tells me: > EAGAIN No more free local ports or insufficient entries in the routing cache. For > AF_INET see the description of > /proc/sys/net/ipv4/ip_local_port_range ip(7) > for information on how to increase the number of local > ports. > > So, the problem is that we took a failed connection as having been > initially successfull but in progress. > > Not accepting EWOULDBLOCK in the above if() results in: > could not connect to server: Resource temporarily unavailable > Is the server running locally and accepting > connections on Unix domain socket "/tmp/.s.PGSQL.5440"? > > which makes more sense. > > Trivial patch attached. Could I convince a committer to NACK or commit & backpatch that patch? It has come up before: http://www.postgresql.org/message-id/CAMnJ+Beq0hCBuTY_=nz=ru0U-No543_RAEunLVSAYU8tugd6NA@mail.gmail.com possibly also: http://lists.pgfoundry.org/pipermail/pgpool-general/2007-March/000595.html Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2013-06-17 16:16:22 +0200, Andres Freund wrote: >> Not accepting EWOULDBLOCK in the above if() results in: >> could not connect to server: Resource temporarily unavailable >> Is the server running locally and accepting >> connections on Unix domain socket "/tmp/.s.PGSQL.5440"? >> which makes more sense. > Could I convince a committer to NACK or commit & backpatch that patch? Some trawling in the commit history shows that the current logic dates from my commit 6d0d838cebdf2bcd5c03f5449a1888f1e120496f, which unified Windows and non-Windows code paths; the check for EWOULDBLOCK was added in the earlier commit ca5a51627919c6fb6ab5e23739615a674caa4037 which (claimed to) add support for non-blocking connect on Windows. So I'm concerned that your patch could break that platform. A possible compromise is { if (SOCK_ERRNO == EINPROGRESS || +#ifndef WIN32 SOCK_ERRNO == EWOULDBLOCK || +#endif SOCK_ERRNO == EINTR || SOCK_ERRNO == 0) but I wonder whether it's safe to remove the case altogether. Could anyone research the situation for non-blocking connect() on Windows? Perhaps on Windows we shouldn't test for EINPROGRESS at all? I'm also pretty suspicious of the SOCK_ERRNO == 0 case, now that I look at it, especially in view of the lack of any attempt to set errno to 0 before the call. The Single Unix Spec is pretty clear that EINTR and EINPROGRESS are the only legit cases, so unless Windows is doing its own thing, we could probably get rid of that too. regards, tom lane
On 2013-06-26 12:07:54 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2013-06-17 16:16:22 +0200, Andres Freund wrote: > >> Not accepting EWOULDBLOCK in the above if() results in: > >> could not connect to server: Resource temporarily unavailable > >> Is the server running locally and accepting > >> connections on Unix domain socket "/tmp/.s.PGSQL.5440"? > >> which makes more sense. > > > Could I convince a committer to NACK or commit & backpatch that patch? > > Some trawling in the commit history shows that the current logic dates > from my commit 6d0d838cebdf2bcd5c03f5449a1888f1e120496f, which unified > Windows and non-Windows code paths; the check for EWOULDBLOCK was added > in the earlier commit ca5a51627919c6fb6ab5e23739615a674caa4037 which > (claimed to) add support for non-blocking connect on Windows. So I'm > concerned that your patch could break that platform. > A possible compromise is > > { > if (SOCK_ERRNO == EINPROGRESS || > +#ifndef WIN32 > SOCK_ERRNO == EWOULDBLOCK || > +#endif > SOCK_ERRNO == EINTR || > SOCK_ERRNO == 0) > > but I wonder whether it's safe to remove the case altogether. Could > anyone research the situation for non-blocking connect() on Windows? > Perhaps on Windows we shouldn't test for EINPROGRESS at all? The way EWOULDBLOCK is mentioned on msdn (http://msdn.microsoft.com/en-us/library/windows/desktop/ms737625%28v=vs.85%29.aspx) it indeed seems to be required. I don't see how we could trigger the conditions for EINPROGRESS on windows that msdn lists, but since we need it on unixoid systems and its valid to treat the connect as partiall successfull on windows, there seems little benefit in dropping it. So I guess the #ifdef you suggested is the way to go. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2013-06-26 12:07:54 -0400, Tom Lane wrote: >> ... I wonder whether it's safe to remove the case altogether. Could >> anyone research the situation for non-blocking connect() on Windows? >> Perhaps on Windows we shouldn't test for EINPROGRESS at all? > The way EWOULDBLOCK is mentioned on msdn > (http://msdn.microsoft.com/en-us/library/windows/desktop/ms737625%28v=vs.85%29.aspx) > it indeed seems to be required. Yeah, that seems pretty clear: WSAEWOULDBLOCK is what's returned when a nonblocking connect() has been started successfully. > I don't see how we could trigger the conditions for EINPROGRESS on > windows that msdn lists, but since we need it on unixoid systems and its > valid to treat the connect as partiall successfull on windows, there > seems little benefit in dropping it. I was about to argue for removing the EINPROGRESS check on Windows, on the grounds that this would be the same type of bug as you're complaining of on Linux, ie, if the call does return this error we'll mistakenly think the connection is in progress and go on to deliver an unhelpful failure message later. However, some more trolling of the intertubes suggests that Cygwin's emulation of socket() does indeed return EINPROGRESS; see for instance this ancient thread of ours: http://www.postgresql.org/message-id/flat/14855.49635.565990.716645@kryten.bedford.waii.com#14855.49635.565990.716645@kryten.bedford.waii.com Unless we want to distinguish Cygwin from native Windows in this code chunk, maybe we'd better leave well enough alone. I still want to delete the test for SOCK_ERRNO == 0. I traced that back to commit da9501bddb42222dc33c031b1db6ce2133bcee7b, but I can't find anything in the mailing list archives to explain that. I have an inquiry in to Jan to see if he can remember the reason ... regards, tom lane
On 2013-06-26 20:07:40 -0400, Tom Lane wrote: > > I don't see how we could trigger the conditions for EINPROGRESS on > > windows that msdn lists, but since we need it on unixoid systems and its > > valid to treat the connect as partiall successfull on windows, there > > seems little benefit in dropping it. > I was about to argue for removing the EINPROGRESS check on Windows, > on the grounds that this would be the same type of bug as you're > complaining of on Linux, ie, if the call does return this error we'll > mistakenly think the connection is in progress and go on to deliver an > unhelpful failure message later. > However, some more trolling of the intertubes suggests that Cygwin's > emulation of socket() does indeed return EINPROGRESS; see for instance > this ancient thread of ours: > http://www.postgresql.org/message-id/flat/14855.49635.565990.716645@kryten.bedford.waii.com#14855.49635.565990.716645@kryten.bedford.waii.com > Unless we want to distinguish Cygwin from native Windows in this code > chunk, maybe we'd better leave well enough alone. After some looking it gets funnier. There currently doesn't seem to be any chance that we actually can get an EINPROGRESS at that level on windows. We #define connect() to pgwin32_connect() which fudges errno around. Where WSAEINPROGRESS is mapped to EINVAL. Also for some reason the connect is performed synchronously to the outside by doing pgwin32_waitforsinglesocket() if EWOULDBLOCK is returned by connect()... That seems to be the case since a4c40f140d23cefbf94e00283f7688c772867f1b. > I still want to delete the test for SOCK_ERRNO == 0. I traced that back > to commit da9501bddb42222dc33c031b1db6ce2133bcee7b, but I can't find > anything in the mailing list archives to explain that. I have an > inquiry in to Jan to see if he can remember the reason ... That looks strange, yes. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2013-06-26 20:07:40 -0400, Tom Lane wrote: >> However, some more trolling of the intertubes suggests that Cygwin's >> emulation of socket() does indeed return EINPROGRESS; see for instance >> this ancient thread of ours: >> http://www.postgresql.org/message-id/flat/14855.49635.565990.716645@kryten.bedford.waii.com#14855.49635.565990.716645@kryten.bedford.waii.com >> Unless we want to distinguish Cygwin from native Windows in this code >> chunk, maybe we'd better leave well enough alone. > After some looking it gets funnier. There currently doesn't seem to be > any chance that we actually can get an EINPROGRESS at that level on > windows. We #define connect() to pgwin32_connect() which fudges errno > around. Where WSAEINPROGRESS is mapped to EINVAL. That's only in the backend though: note the #define is controlled by #ifndef FRONTEND, and we don't link backend/port/win32/socket.c into libpq anyway. The signal managing it's doing wouldn't work at all in client-side programs. But yeah, on the backend side we would definitely treat WSAEINPROGRESS as a hard error. OTOH, we don't use nonblocking connect (much?) in the backend so I'm not sure whether that's a directly comparable case or not. regards, tom lane
Andres Freund <andres@2ndquadrant.com> writes: > On 2013-06-26 20:07:40 -0400, Tom Lane wrote: >> I still want to delete the test for SOCK_ERRNO == 0. I traced that back >> to commit da9501bddb42222dc33c031b1db6ce2133bcee7b, but I can't find >> anything in the mailing list archives to explain that. I have an >> inquiry in to Jan to see if he can remember the reason ... > That looks strange, yes. Committed that way. We can always undo it if Jan can defend the logic. regards, tom lane