couple more bugs survived

* server connection was released too early, there were unsent data.
* put logged in clients immidiately to pause if SUSPEND.
* pause_mode cleanup

diff --git a/NEWS b/NEWS
index bb1ea87db779f2f9c750daab33a748ab9770e049..094f705c25882d308d83dca54abf0434da0b8692 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -8,11 +8,13 @@
   * Fix rare case when socket woken up from send-wait could stay stalling.
   * More fair queueing of server connections.  Before, a new query could
     get a server connections before older one.
+  * Delay server release until everything is guaranteed to be sent.
 
   = Features =
 
   * SHOW SOCKETS command to have detailed info about state state.
   * Put PgSocket ptr to log, to help tracking one connection.
+  * Various code cleanups.
 
 2007-03-13  -  PgBouncer 1.0  -  "Tuunitud bemm"
 

diff --git a/src/admin.c b/src/admin.c
index 99b1dcf0dcabb96d3a034bccd3f0cd2303815a36..c08e21b24a52e31adf5c68a0053cd872af537957 100644 (file)
--- a/src/admin.c
+++ b/src/admin.c
@@ -584,11 +584,11 @@ static bool admin_cmd_resume(PgSocket *admin)
                return admin_error(admin, "admin access needed");
 
        log_info("RESUME command issued");
-       cf_pause_mode = 0;
+       cf_pause_mode = P_NONE;
        switch (tmp_mode) {
-       case 2:
+       case P_SUSPEND:
                resume_all();
-       case 1:
+       case P_PAUSE:
                return admin_ready(admin, "RESUME");
        default:
                return admin_error(admin, "Pooler is not paused/suspended");
@@ -605,7 +605,7 @@ static bool admin_cmd_suspend(PgSocket *admin)
                return admin_error(admin, "already suspended/paused");
 
        log_info("SUSPEND command issued");
-       cf_pause_mode = 2;
+       cf_pause_mode = P_SUSPEND;
        admin->wait_for_response = 1;
        suspend_pooler();
 
@@ -622,7 +622,7 @@ static bool admin_cmd_pause(PgSocket *admin)
                return admin_error(admin, "already suspended/paused");
 
        log_info("PAUSE command issued");
-       cf_pause_mode = 1;
+       cf_pause_mode = P_PAUSE;
        admin->wait_for_response = 1;
 
        return true;
@@ -911,10 +911,10 @@ void admin_pause_done(void)
                        continue;
 
                switch (cf_pause_mode) {
-               case 1:
+               case P_PAUSE:
                        admin_ready(admin, "PAUSE");
                        break;
-               case 2:
+               case P_SUSPEND:
                        admin_ready(admin, "SUSPEND");
                        break;
                default:
@@ -924,10 +924,10 @@ void admin_pause_done(void)
        }
 
        if (statlist_empty(&admin_pool->active_client_list)
-                       && cf_pause_mode == 2)
+                       && cf_pause_mode == P_SUSPEND)
        {
                log_info("Admin disappeared when suspended, doing RESUME");
-               cf_pause_mode = 0;
+               cf_pause_mode = P_NONE;
                resume_all();
        }
 }

diff --git a/src/bouncer.h b/src/bouncer.h
index 264ccec05c277c0e52a4f2e428e150007165edfd..5d4fc852aaac64f0650c6e6f99c8fe48e14092b8 100644 (file)
--- a/src/bouncer.h
+++ b/src/bouncer.h
@@ -40,6 +40,12 @@ enum SocketState {
        SV_TESTED               /* pool->tested_server_list */
 };
 
+enum PauseMode {
+       P_NONE = 0,
+       P_PAUSE = 1,
+       P_SUSPEND = 2
+};
+
 #define is_server_socket(sk) ((sk)->state >= SV_FREE)
 
 

diff --git a/src/client.c b/src/client.c
index bf48d712f0ee0bbbdac7b56b4edc71a1002d1cd6..5abb916b4e4af1732514cd5b0e85495f266da3c9 100644 (file)
--- a/src/client.c
+++ b/src/client.c
@@ -375,6 +375,10 @@ bool client_proto(SBuf *sbuf, SBufEvent evtype, MBuf *pkt, void *arg)
                default:
                        fatal("bad client state: %d", client->state);
                }
+               break;
+       case SBUF_EV_FLUSH:
+               /* client is not interested in it */
+               break;
        }
        return res;
 }

diff --git a/src/janitor.c b/src/janitor.c
index 1cde69f8a3d5200dcad60e60a9baca09be7a1187..57d2d8d851786e50a160a9e5d139557286000362 100644 (file)
--- a/src/janitor.c
+++ b/src/janitor.c
@@ -125,7 +125,7 @@ static void launch_recheck(PgPool *pool)
                        disconnect_server(server, false, "test query failed");
        } else
                /* make immidiately available */
-               change_server_state(server, SV_IDLE);
+               release_server(server);
 }
 
 /*
@@ -204,9 +204,9 @@ static int per_loop_suspend(PgPool *pool)
        if (!active) {
                active += suspend_socket_list(&pool->active_server_list);
                active += suspend_socket_list(&pool->idle_server_list);
-               active += statlist_count(&pool->tested_server_list);
 
                /* as all clients are done, no need for them */
+               close_server_list(&pool->tested_server_list, "close unsafe fds on suspend");
                close_server_list(&pool->used_server_list, "close unsafe fds on suspend");
        }
 
@@ -227,22 +227,22 @@ void per_loop_object_maint(void)
                if (pool->admin)
                        continue;
                switch (cf_pause_mode) {
-               case 0:
+               case P_NONE:
                        per_loop_activate(pool);
                        break;
-               case 1:
+               case P_PAUSE:
                        active += per_loop_pause(pool);
                        break;
-               case 2:
+               case P_SUSPEND:
                        active += per_loop_suspend(pool);
                        break;
                }
        }
 
        switch (cf_pause_mode) {
-       case 2:
+       case P_SUSPEND:
                active += statlist_count(&login_client_list);
-       case 1:
+       case P_PAUSE:
                if (!active)
                        admin_pause_done();
        default:
@@ -305,7 +305,7 @@ static void check_unused_servers(StatList *slist, usec_t now, bool idle_test)
                        disconnect_server(server, true, "server idle timeout");
                else if (cf_server_lifetime > 0 && age > cf_server_lifetime)
                        disconnect_server(server, true, "server lifetime over");
-               else if (cf_pause_mode == 1)
+               else if (cf_pause_mode == P_PAUSE)
                        disconnect_server(server, true, "pause mode");
                else if (idle_test && *cf_server_check_query) {
                        if (idle > cf_server_check_delay)

diff --git a/src/main.c b/src/main.c
index fadd00846cc366e2d65f7a0e02b5ec619ac070bc..9fdffc3e85629fbba872eaaff6a596cf892deb4d 100644 (file)
--- a/src/main.c
+++ b/src/main.c
@@ -45,7 +45,7 @@ static void usage(int err)
 
 int cf_verbose = 0;
 int cf_daemon = 0;
-int cf_pause_mode = 0;
+int cf_pause_mode = P_NONE;
 int cf_shutdown = 0;
 int cf_reboot = 0;
 static char *cf_config_file;
@@ -236,7 +236,7 @@ static void handle_sigterm(int sock, short flags, void *arg)
 static void handle_sigint(int sock, short flags, void *arg)
 {
        log_info("Got SIGINT, shutting down");
-       cf_pause_mode = 1;
+       cf_pause_mode = P_PAUSE;
        cf_shutdown = 1;
 }
 
@@ -244,7 +244,7 @@ static void handle_sigusr1(int sock, short flags, void *arg)
 {
        if (cf_pause_mode == 0) {
                log_info("Got SIGUSR1, pausing all activity");
-               cf_pause_mode = 1;
+               cf_pause_mode = P_PAUSE;
        } else {
                log_info("Got SIGUSR1, but already paused/suspended");
        }
@@ -253,16 +253,16 @@ static void handle_sigusr1(int sock, short flags, void *arg)
 static void handle_sigusr2(int sock, short flags, void *arg)
 {
        switch (cf_pause_mode) {
-       case 2:
+       case P_SUSPEND:
                log_info("Got SIGUSR2, continuing from SUSPEND");
                resume_all();
                cf_pause_mode = 0;
                break;
-       case 1:
+       case P_PAUSE:
                log_info("Got SIGUSR2, continuing from PAUSE");
                cf_pause_mode = 0;
                break;
-       case 0:
+       case P_NONE:
                log_info("Got SIGUSR1, but not paused/suspended");
        }
 }

diff --git a/src/objects.c b/src/objects.c
index 7131d81c4db5e1fdce571ed3436b328d22157967..7222af348c8930d8aca2d8db64fee74752259be2 100644 (file)
--- a/src/objects.c
+++ b/src/objects.c
@@ -503,7 +503,7 @@ bool find_server(PgSocket *client)
                return true;
 
        /* try to get idle server, if allowed */
-       if (cf_pause_mode == 1)
+       if (cf_pause_mode == P_PAUSE)
                server = NULL;
        else
                server = first_socket(&pool->idle_server_list);
@@ -748,13 +748,18 @@ bool finish_client_login(PgSocket *client)
                log_debug("finish_client_login: no welcome msg, pause");
                client->wait_for_welcome = 1;
                pause_client(client);
-               if (!cf_pause_mode)
+               if (cf_pause_mode == P_NONE)
                        launch_new_connection(client->pool);
                return false;
        }
        client->wait_for_welcome = 0;
 
        slog_debug(client, "logged in");
+
+       /* in suspend, dont let send query */
+       if (cf_pause_mode == P_SUSPEND)
+               pause_client(client);
+
        return true;
 }
 

diff --git a/src/sbuf.c b/src/sbuf.c
index 385182cfff4c98c9d0ea73444f2a9cfc09b4a0eb..a3a636f7eb3e6cabc6a58384afb3703b2e8890a3 100644 (file)
--- a/src/sbuf.c
+++ b/src/sbuf.c
@@ -446,6 +446,13 @@ try_more:
        /* if the buffer is full, there can be more data available */
        if (ok && sbuf->recv_pos == cf_sbuf_len)
                goto try_more;
+
+       /* clean buffer */
+       sbuf_try_resync(sbuf);
+
+       /* notify proto that all is sent */
+       if (sbuf->send_pos == sbuf->recv_pos && sbuf->pkt_remain == 0)
+               sbuf_call_proto(sbuf, SBUF_EV_FLUSH);
 }
 
 /* check if there is any error pending on socket */

diff --git a/src/sbuf.h b/src/sbuf.h
index 6c1b11c02a13a6fca8d2d71f45a097ad86e65690..615765e5111b85d9d3d04154278400e73c62cdef 100644 (file)
--- a/src/sbuf.h
+++ b/src/sbuf.h
@@ -21,7 +21,8 @@ typedef enum {
        SBUF_EV_RECV_FAILED,
        SBUF_EV_SEND_FAILED,
        SBUF_EV_CONNECT_FAILED,
-       SBUF_EV_CONNECT_OK
+       SBUF_EV_CONNECT_OK,
+       SBUF_EV_FLUSH
 } SBufEvent;
 
 typedef struct SBuf SBuf;

diff --git a/src/server.c b/src/server.c
index e09abc7ff9b96841acd8e7302e7e10d7a45ece24..9c6c28de7c9109cd6f8ef193c9bfdb73ebd8bb42 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -196,10 +196,6 @@ static bool handle_server_work(PgSocket *server, MBuf *pkt)
                slog_debug(client, "query time: %d us", (int)total);
        }
 
-       if (ready && (     cf_pool_mode  != POOL_SESSION
-                       || server->state == SV_TESTED))
-               release_server(server);
-
        return true;
 }
 
@@ -271,6 +267,25 @@ bool server_proto(SBuf *sbuf, SBufEvent evtype, MBuf *pkt, void *arg)
                Assert(server->state == SV_LOGIN);
                server->request_time = get_cached_time();
                res = handle_connect(server);
+               break;
+       case SBUF_EV_FLUSH:
+               if (server->ready
+                   && (cf_pool_mode  != POOL_SESSION
+                       || server->state == SV_TESTED))
+               {
+                       switch (server->state) {
+                       case SV_ACTIVE:
+                       case SV_TESTED:
+                               release_server(server);
+                               break;
+                       default:
+                               slog_warning(server, "EV_FLUSH with state=%d", server->state);
+                       case SV_IDLE:
+                               break;
+                       }
+               }
+               res = true; /* unused actually */
+               break;
        }
        return res;
 }