From: Eric Wong <normalperson@yhbt•net>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org
Subject: [PATCH] attempt connects in parallel for IPv6-capable builds
Date: Thu, 28 Jan 2016 11:57:21 +0000 [thread overview]
Message-ID: <20160128115720.GA1827@dcvr.yhbt.net> (raw)
getaddrinfo() may return multiple addresses, not all of which
are equally performant. In some cases, a user behind a non-IPv6
capable network may get an IPv6 address which stalls connect().
Instead of waiting synchronously for a connect() to timeout, use
non-blocking connect() in parallel and take the first successful
connection.
This may increase network traffic and server load slightly, but
makes the worst-case user experience more bearable when one
lacks permissions to edit /etc/gai.conf to favor IPv4 addresses.
Signed-off-by: Eric Wong <normalperson@yhbt•net>
---
connect.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 104 insertions(+), 14 deletions(-)
diff --git a/connect.c b/connect.c
index fd7ffe1..74d2bb5 100644
--- a/connect.c
+++ b/connect.c
@@ -14,6 +14,42 @@
static char *server_capabilities;
static const char *parse_feature_value(const char *, const char *, int *);
+#ifdef SOCK_NONBLOCK /* Linux-only flag */
+# define GIT_SOCK_NONBLOCK SOCK_NONBLOCK
+#else
+# define GIT_SOCK_NONBLOCK 0
+#endif
+
+static int socket_nb(int domain, int type, int protocol)
+{
+ static int flags = GIT_SOCK_NONBLOCK;
+ int fd = socket(domain, type | flags, protocol);
+
+ /* new headers, old kernel? */
+ if (fd < 0 && errno == EINVAL && flags != 0) {
+ flags = 0;
+ fd = socket(domain, type, protocol);
+ }
+
+ /* couldn't use SOCK_NONBLOCK, set non-blocking the old way */
+ if (flags == 0 && fd >= 0) {
+ int fl = fcntl(fd, F_GETFL);
+
+ if (fcntl(fd, F_SETFL, fl | O_NONBLOCK) < 0)
+ die_errno("failed to set nonblocking flag\n");
+ }
+
+ return fd;
+}
+
+static void set_blocking(int fd)
+{
+ int fl = fcntl(fd, F_GETFL);
+
+ if (fcntl(fd, F_SETFL, fl & ~O_NONBLOCK) < 0)
+ die_errno("failed to clear nonblocking flag\n");
+}
+
static int check_ref(const char *name, unsigned int flags)
{
if (!flags)
@@ -351,6 +387,9 @@ static int git_tcp_connect_sock(char *host, int flags)
struct addrinfo hints, *ai0, *ai;
int gai;
int cnt = 0;
+ nfds_t n = 0, nfds = 0;
+ struct pollfd *fds = NULL;
+ struct addrinfo **inprogress = NULL;
get_host_and_port(&host, &port);
if (!*port)
@@ -371,20 +410,76 @@ static int git_tcp_connect_sock(char *host, int flags)
fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
for (ai0 = ai; ai; ai = ai->ai_next, cnt++) {
- sockfd = socket(ai->ai_family,
- ai->ai_socktype, ai->ai_protocol);
- if ((sockfd < 0) ||
- (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0)) {
+ size_t cur;
+ int fd = socket_nb(ai->ai_family, ai->ai_socktype,
+ ai->ai_protocol);
+ if (fd < 0) {
strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
host, cnt, ai_name(ai), strerror(errno));
- if (0 <= sockfd)
- close(sockfd);
- sockfd = -1;
continue;
}
+
+ if (connect(fd, ai->ai_addr, ai->ai_addrlen) < 0 &&
+ errno != EINPROGRESS) {
+ strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
+ host, cnt, ai_name(ai), strerror(errno));
+ close(fd);
+ continue;
+ }
+
if (flags & CONNECT_VERBOSE)
- fprintf(stderr, "%s ", ai_name(ai));
- break;
+ fprintf(stderr, "%s (started)\n", ai_name(ai));
+
+ nfds = n + 1;
+ cur = n;
+ ALLOC_GROW(fds, nfds, cur);
+ cur = n;
+ ALLOC_GROW(inprogress, nfds, cur);
+ inprogress[n] = ai;
+ fds[n].fd = fd;
+ fds[n].events = POLLIN|POLLOUT;
+ fds[n].revents = 0;
+ n = nfds;
+ }
+
+ /*
+ * nfds is tiny, no need to limit loop based on poll() retval,
+ * just do not let poll sleep forever if nfds is zero
+ */
+ if (nfds > 0)
+ poll(fds, nfds, -1);
+
+ for (n = 0; n < nfds && sockfd < 0; n++) {
+ if (fds[n].revents & (POLLERR|POLLHUP))
+ continue;
+ if (fds[n].revents & POLLOUT) {
+ int err;
+ socklen_t len = (socklen_t)sizeof(err);
+ int rc = getsockopt(fds[n].fd, SOL_SOCKET, SO_ERROR,
+ &err, &len);
+ if (rc != 0)
+ die_errno("getsockopt errno=%s\n",
+ strerror(errno));
+ if (err == 0) { /* success! */
+ sockfd = fds[n].fd;
+ ai = inprogress[n];
+ }
+ }
+ }
+
+ /* cleanup */
+ for (n = 0; n < nfds; n++) {
+ if (fds[n].fd != sockfd)
+ close(fds[n].fd);
+ }
+ free(inprogress);
+ free(fds);
+
+ if (sockfd >= 0) {
+ enable_keepalive(sockfd);
+ set_blocking(sockfd); /* the rest of git expects blocking */
+ if (flags & CONNECT_VERBOSE)
+ fprintf(stderr, "%s done.\n", ai_name(ai));
}
freeaddrinfo(ai0);
@@ -392,11 +487,6 @@ static int git_tcp_connect_sock(char *host, int flags)
if (sockfd < 0)
die("unable to connect to %s:\n%s", host, error_message.buf);
- enable_keepalive(sockfd);
-
- if (flags & CONNECT_VERBOSE)
- fprintf(stderr, "done.\n");
-
strbuf_release(&error_message);
return sockfd;
--
EW
next reply other threads:[~2016-01-28 11:57 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-28 11:57 Eric Wong [this message]
2016-01-28 23:42 ` [PATCH] attempt connects in parallel for IPv6-capable builds Junio C Hamano
2016-01-29 1:41 ` Eric Wong
2016-01-29 7:10 ` Johannes Sixt
2016-01-29 3:04 ` Junio C Hamano
2016-01-29 6:08 ` Torsten Bögershausen
2016-01-30 13:09 ` Eric Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160128115720.GA1827@dcvr.yhbt.net \
--to=normalperson@yhbt$(echo .)net \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox