MINOR: connection: avoid a useless recvfrom() on outgoing connections
When a connect() doesn't immediately succeed (i.e. most of the times),
fd_cant_send() is called to enable polling. But given that we don't
mark that we cannot receive either, we end up performing a failed
recvfrom() immediately when the connect() is finally confirmed, as
indicated in issue #253.
This patch simply adds fd_cant_recv() as well so that we're only
notified once the recv path is ready. The reason it was not there
is purely historic, as in the past when there was the fd cache,
doing it would have caused a pending recv request to be placed into
the fd cache, hence a useless recvfrom() upon success (i.e. what
happens now).
Without this patch, forwarding 100k connections does this:
% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ----------------
17.51 0.704229 7 100000 100000 connect
16.75 0.673875 3 200000 sendto
16.24 0.653222 3 200036 close
10.82 0.435082 1 300000 100000 recvfrom
10.37 0.417266 1 300012 setsockopt
7.12 0.286511 1 199954 epoll_ctl
6.80 0.273447 2 100000 shutdown
5.34 0.214942 2 100005 socket
4.65 0.187137 1 105002 5002 accept4
3.35 0.134757 1 100004 fcntl
0.61 0.024585 4 5858 epoll_wait
With the patch:
% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ----------------
18.04 0.697365 6 100000 100000 connect
17.40 0.672471 3 200000 sendto
17.03 0.658134 3 200036 close
10.57 0.408459 1 300012 setsockopt
7.69 0.297270 1 200000 recvfrom
7.32 0.282934 1 199922 epoll_ctl
7.09 0.274027 2 100000 shutdown
5.59 0.216041 2 100005 socket
4.87 0.188352 1 104697 4697 accept4
3.35 0.129641 1 100004 fcntl
0.65 0.024959 4 5337 1 epoll_wait
Note the total disappearance of 1/3 of failed recvfrom() *without*
adding any extra syscall anywhere else.
The trace of an HTTP health check is now totally clean, with no useless
syscall at all anymore:
09:14:21.959255 connect(9, {sa_family=AF_INET, sin_port=htons(8000), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
09:14:21.959292 epoll_ctl(4, EPOLL_CTL_ADD, 9, {EPOLLIN|EPOLLOUT|EPOLLRDHUP, {u32=9, u64=9}}) = 0
09:14:21.959315 epoll_wait(4, [{EPOLLOUT, {u32=9, u64=9}}], 200, 1000) = 1
09:14:21.959376 sendto(9, "OPTIONS / HTTP/1.0\r\ncontent-leng"..., 41, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 41
09:14:21.959436 epoll_wait(4, [{EPOLLOUT, {u32=9, u64=9}}], 200, 1000) = 1
09:14:21.959456 epoll_ctl(4, EPOLL_CTL_MOD, 9, {EPOLLIN|EPOLLRDHUP, {u32=9, u64=9}}) = 0
09:14:21.959512 epoll_wait(4, [{EPOLLIN|EPOLLRDHUP, {u32=9, u64=9}}], 200, 1000) = 1
09:14:21.959548 recvfrom(9, "HTTP/1.0 200\r\nContent-length: 0\r"..., 16320, 0, NULL, NULL) = 126
09:14:21.959570 close(9) = 0
With the edge-triggered poller, it gets even better:
09:29:15.776201 connect(9, {sa_family=AF_INET, sin_port=htons(8000), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
09:29:15.776256 epoll_ctl(4, EPOLL_CTL_ADD, 9, {EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLET, {u32=9, u64=9}}) = 0
09:29:15.776287 epoll_wait(4, [{EPOLLOUT, {u32=9, u64=9}}], 200, 1000) = 1
09:29:15.776320 sendto(9, "OPTIONS / HTTP/1.0\r\ncontent-leng"..., 41, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 41
09:29:15.776374 epoll_wait(4, [{EPOLLIN|EPOLLOUT|EPOLLRDHUP, {u32=9, u64=9}}], 200, 1000) = 1
09:29:15.776406 recvfrom(9, "HTTP/1.0 200\r\nContent-length: 0\r"..., 16320, 0, NULL, NULL) = 126
09:29:15.776434 close(9) = 0
It could make sense to backport this patch to 2.2 and maybe 2.1 after
it has been sufficiently checked for absence of side effects in 2.3-dev,
as some people had reported an extra overhead like in issue #168.
diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index 700b91d..586b119 100644
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -588,6 +588,7 @@
if (conn->flags & CO_FL_WAIT_L4_CONN) {
fd_want_send(fd);
fd_cant_send(fd);
+ fd_cant_recv(fd);
}
if (conn_xprt_init(conn) < 0) {