From: Marko Kreen <markokr@gmail.com>
Date: Mon, 7 Jan 2008 09:02:09 +0000 (+0000)
Subject: safe_accept, safe_connect, safe_evtimer_add
X-Git-Tag: pgbouncer_1_2_rc2~87
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2625f3f78dd131470ee9533bcedec142b27a4713;p=pgbouncer

safe_accept, safe_connect, safe_evtimer_add
---

diff --git a/include/util.h b/include/util.h
index e721069..71a17de 100644
--- a/include/util.h
+++ b/include/util.h
@@ -86,6 +86,8 @@ int safe_send(int fd, const void *buf, int len, int flags) 	_MUSTCHECK;
 int safe_close(int fd);
 int safe_recvmsg(int fd, struct msghdr *msg, int flags)		_MUSTCHECK;
 int safe_sendmsg(int fd, const struct msghdr *msg, int flags)	_MUSTCHECK;
+int safe_connect(int fd, const struct sockaddr *sa, socklen_t sa_len)	_MUSTCHECK;
+int safe_accept(int fd, struct sockaddr *sa, socklen_t *sa_len)	_MUSTCHECK;
 
 /*
  * password tools
@@ -107,3 +109,6 @@ void fill_remote_addr(PgSocket *sk, int fd, bool is_unix);
 void fill_local_addr(PgSocket *sk, int fd, bool is_unix);
 
 
+void rescue_timers(void);
+void safe_evtimer_add(struct event *ev, struct timeval *tv);
+
diff --git a/src/janitor.c b/src/janitor.c
index 7d8800b..06fd710 100644
--- a/src/janitor.c
+++ b/src/janitor.c
@@ -456,7 +456,6 @@ static void do_full_maint(int sock, short flags, void *arg)
 {
 	List *item;
 	PgPool *pool;
-	int err;
 
 	/* don't touch anything if takeover is in progress */
 	if (cf_reboot)
@@ -481,22 +480,15 @@ static void do_full_maint(int sock, short flags, void *arg)
 		loader_users_check();
 
 skip:
-	err = evtimer_add(&full_maint_ev, &full_maint_period);
-	if (err < 0)
-		/* fixme */
-		fatal_perror("evtimer_add");
+	safe_evtimer_add(&full_maint_ev, &full_maint_period);
 }
 
 /* first-time initializtion */
 void janitor_setup(void)
 {
-	int err;
-
 	/* launch maintenance */
 	evtimer_set(&full_maint_ev, do_full_maint, NULL);
-	err = evtimer_add(&full_maint_ev, &full_maint_period);
-	if (err < 0)
-		fatal_perror("janitor_setup: evtimer_add");
+	safe_evtimer_add(&full_maint_ev, &full_maint_period);
 }
 
 /* as [pgbouncer] section can be loaded after databases,
diff --git a/src/main.c b/src/main.c
index 9f6708d..769c075 100644
--- a/src/main.c
+++ b/src/main.c
@@ -482,6 +482,7 @@ static void main_loop_once(void)
 	}
 	per_loop_maint();
 	reuse_just_freed_objects();
+	rescue_timers();
 }
 
 /* boot everything */
diff --git a/src/pooler.c b/src/pooler.c
index f65f672..fcbc659 100644
--- a/src/pooler.c
+++ b/src/pooler.c
@@ -172,7 +172,7 @@ static void err_wait_func(int sock, short flags, void *arg)
 /* got new connection, associate it with client struct */
 static void pool_accept(int sock, short flags, void *is_unix)
 {
-	int fd, err;
+	int fd;
 	PgSocket *client;
 	union {
 		struct sockaddr_in in;
@@ -183,11 +183,9 @@ static void pool_accept(int sock, short flags, void *is_unix)
 
 loop:
 	/* get fd */
-	fd = accept(sock, &addr.sa, &len);
+	fd = safe_accept(sock, &addr.sa, &len);
 	if (fd < 0) {
-		if (errno == EINTR)
-			goto loop;
-		else if (errno == EAGAIN)
+		if (errno == EAGAIN)
 			return;
 		else if (errno == ECONNABORTED)
 			return;
@@ -198,11 +196,8 @@ loop:
 		 */
 		log_error("accept() failed: %s", strerror(errno));
 		evtimer_set(&ev_err, err_wait_func, NULL);
-		err = evtimer_add(&ev_err, &err_timeout);
-		if (err < 0)
-			log_error("pool_accept: evtimer_add: %s", strerror(errno));
-		else
-			suspend_pooler();
+		safe_evtimer_add(&ev_err, &err_timeout);
+		suspend_pooler();
 		return;
 	}
 
diff --git a/src/sbuf.c b/src/sbuf.c
index da7198c..b7a598e 100644
--- a/src/sbuf.c
+++ b/src/sbuf.c
@@ -153,9 +153,7 @@ bool sbuf_connect(SBuf *sbuf, const PgAddr *addr, const char *unix_dir, int time
 	timeout.tv_usec = 0;
 
 	/* launch connection */
-loop:
-	res = connect(sock, sa, len);
-	log_noise("connect(%d)=%d", sock, res);
+	res = safe_connect(sock, sa, len);
 	if (res == 0) {
 		/* unix socket gives connection immidiately */
 		sbuf_connect_cb(sock, EV_WRITE, sbuf);
@@ -166,8 +164,6 @@ loop:
 		res = event_add(&sbuf->ev, &timeout);
 		if (res >= 0)
 			return true;
-	} else if (errno == EINTR) {
-		goto loop;
 	}
 
 failed:
diff --git a/src/stats.c b/src/stats.c
index 3b6cd86..2ade3db 100644
--- a/src/stats.c
+++ b/src/stats.c
@@ -128,7 +128,6 @@ static void refresh_stats(int s, short flags, void *arg)
 	PgPool *pool;
 	struct timeval period = { cf_stats_period, 0 };
 	PgStats old_total, cur_total, avg;
-	int err;
 
 	reset_stats(&old_total);
 	reset_stats(&cur_total);
@@ -151,26 +150,18 @@ static void refresh_stats(int s, short flags, void *arg)
 		 (ull_t)avg.request_count, (ull_t)avg.client_bytes,
 		 (ull_t)avg.server_bytes, (ull_t)avg.query_time);
 
-	err = evtimer_add(&ev_stats, &period);
-	if (err < 0) {
-		/* fixme */
-		log_warning("Cannot re-add stats timer, stats non-functional now: %s",
-			    strerror(errno));
-	}
+	safe_evtimer_add(&ev_stats, &period);
 }
 
 void stats_setup(void)
 {
 	struct timeval period = { cf_stats_period, 0 };
-	int err;
 
 	new_stamp = get_cached_time();
 	old_stamp = new_stamp - USEC;
 
-	/* launch maintenance */
+	/* launch stats */
 	evtimer_set(&ev_stats, refresh_stats, NULL);
-	err = evtimer_add(&ev_stats, &period);
-	if (err < 0)
-		fatal_perror("evtimer_add");
+	safe_evtimer_add(&ev_stats, &period);
 }
 
diff --git a/src/util.c b/src/util.c
index b64c450..45402be 100644
--- a/src/util.c
+++ b/src/util.c
@@ -342,6 +342,49 @@ loop:
 	return res;
 }
 
+static const char *sa2str(const struct sockaddr *sa)
+{
+	static char buf[256];
+	if (sa->sa_family == AF_UNIX) {
+		struct sockaddr_un *un = (struct sockaddr_un *)sa;
+		snprintf(buf, sizeof(buf), "unix:%s", un->sun_path);
+	} else if (sa->sa_family == AF_INET) {
+		struct sockaddr_in *in = (struct sockaddr_in *)sa;
+		snprintf(buf, sizeof(buf), "%s:%d", inet_ntoa(in->sin_addr), ntohs(in->sin_port));
+	} else {
+		snprintf(buf, sizeof(buf), "sa2str: unknown proto");
+	}
+	return buf;
+}
+
+int safe_connect(int fd, const struct sockaddr *sa, socklen_t sa_len)
+{
+	int res;
+loop:
+	res = connect(fd, sa, sa_len);
+	if (res < 0 && errno == EINTR)
+		goto loop;
+	if (res < 0 && (errno != EINPROGRESS || cf_verbose > 2))
+		log_noise("connect(%d, %s) = %s", fd, sa2str(sa), strerror(errno));
+	else if (cf_verbose > 2)
+		log_noise("connect(%d, %s) = %d", fd, sa2str(sa), res);
+	return res;
+}
+
+int safe_accept(int fd, struct sockaddr *sa, socklen_t *sa_len_p)
+{
+	int res;
+loop:
+	res = accept(fd, sa, sa_len_p);
+	if (res < 0 && errno == EINTR)
+		goto loop;
+	if (res < 0)
+		log_noise("safe_accept(%d) = %s", fd, strerror(errno));
+	else if (cf_verbose > 2)
+		log_noise("safe_accept(%d) = %d (%s)", fd, res, sa2str(sa));
+	return res;
+}
+
 /*
  * Load a file into malloc()-ed C string.
  */
@@ -656,3 +699,46 @@ void fill_local_addr(PgSocket *sk, int fd, bool is_unix)
 	}
 }
 
+/*
+ * Error handling around evtimer_add() is nasty as the code
+ * may not be called again.  As there is fixed number of timers
+ * in pgbouncer, provider safe_evtimer_add() that stores args of
+ * failed calls in static array and retries later.
+ */
+#define TIMER_BACKUP_SLOTS  10
+
+struct timer_slot {
+	struct event *ev;
+	struct timeval tv;
+};
+static struct timer_slot timer_backup_list[TIMER_BACKUP_SLOTS];
+static int timer_backup_used = 0;
+
+void safe_evtimer_add(struct event *ev, struct timeval *tv)
+{
+	int res;
+	struct timer_slot *ts;
+	
+	res = evtimer_add(ev, tv);
+	if (res >= 0)
+		return;
+
+	if (timer_backup_used >= TIMER_BACKUP_SLOTS)
+		fatal_perror("TIMER_BACKUP_SLOTS full");
+
+	ts = &timer_backup_list[timer_backup_used++];
+	ts->ev = ev;
+	ts->tv = *tv;
+}
+
+void rescue_timers(void)
+{
+	struct timer_slot *ts;
+	while (timer_backup_used) {
+		ts = &timer_backup_list[timer_backup_used - 1];
+		if (evtimer_add(ts->ev, &ts->tv) < 0)
+			break;
+		timer_backup_used--;
+	}
+}
+