safe_accept, safe_connect, safe_evtimer_add

diff --git a/include/util.h b/include/util.h
index e7210695af3cacb1525206c4c8b78ea1f7b5c544..71a17dedfbc2a69c303f626f71ecd20e16b18d5a 100644 (file)
--- 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 7d8800b12d8a4b462561281d5c59ef68aa33f972..06fd7107380ad46d72f7f53db2bb1c0bb225dda1 100644 (file)
--- 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 9f6708d2bae84cb7c9263bafb8fea850599a354c..769c0753e8fdad180f1f4897c41eaf67942b4227 100644 (file)
--- 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 f65f67262e810e20da65e28706d768d6af8abd24..fcbc6596b607820373a710aa9b054958d7c03c26 100644 (file)
--- 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 da7198c2b4a21ab7b355d742c2a8fec08e74fcc5..b7a598ea58d23c4c2b95b4cc4a7ea4b6dcac4e4f 100644 (file)
--- 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 3b6cd8688f9fad48d3827405f6f72699479d7ea3..2ade3db77555ea25e9acb962a6a755196f85f7e4 100644 (file)
--- 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 b64c4507a2d91c4bb0554545c58d8873ad18edec..45402be711e911d4e0a2ba55a59c9a2bd9adfe00 100644 (file)
--- 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--;
+       }
+}
+