res_odbc: cache_type option for res_odbc.

This enables setting cache_type classes to a round-robin queueing system
rather than the historic stack mechanism.

This should result in lower risk of connection drops due to shorter idle
times (the first connection to go onto the stack could in theory never
be used again, ever, but sit there consuming resources, there could be
multiple of these).

And with a queue rather than a stack, dead connections are guaranteed to
be detected and purged eventually.

This should end up better balancing connection_cnt with actual load
over time, assuming the database doesn't keep connections open
excessively long from it's side.

Signed-off-by: Jaco Kroon <jaco@uls.co.za>
UserNote: When using res_odbc it should be noted that back-end
connections to the underlying database can now be configured to re-use
the cached connections in a round-robin manner rather than repeatedly
re-using the same connection.  This helps to keep connections alive, and
to purge dead connections from the system, thus more dynamically
adjusting to actual load.  The downside is that one could keep too many
connections active for a longer time resulting in resource also begin
consumed on the database side.

diff --git a/configs/samples/res_odbc.conf.sample b/configs/samples/res_odbc.conf.sample
index 58671fc4af235b78a583aa2594f3d2e75bdd5826..277f33148d4f6972618185df3f5452c001f2fd60 100644 (file)
--- a/configs/samples/res_odbc.conf.sample
+++ b/configs/samples/res_odbc.conf.sample
@@ -79,6 +79,24 @@ pre-connect => yes
 ; default this is set to 5000 milliseconds (or 5 seconds). If you would like to
 ; disable the WARNING message it can be set to "0".
 ;slow_query_limit => 5000
+;
+; Set the type of connection cache.  Historically res_odbc used a stack, resulting in
+; a single connection being re-used frequently, whilst other connections could end up
+; dying.  Using a queue or round-robin strategy results in all connections being re-used
+; and kept alive.
+; Each strategy has pros and cons as debated on
+; https://github.com/asterisk/asterisk/pull/1026.  In summary, the stack results
+; in a small number of connections remaining "hot" on the database side which
+; could argumentatively maintain higher performance at the cost of longer-idle
+; connections being timed out from the DB side, resulting in a short CPU spike on
+; asterisk side when that "dead tail" is attempted to be used.  The queue on the
+; other hand will keep the load more spread over connections, resulting in them
+; being kept active, avoiding the CPU spike on asterisk side to a much larger
+; degree, at the cost of potentially keeping more connections (which could
+; potentially be purged DB side) active.
+; The default remains a stack to be backwards compatible, setting this value to
+; any of rr, roundrobin or queue will result in a round-robin queue being used.
+;cache_type => roundrobin
 
 [mysql2]
 enabled => no

diff --git a/res/res_odbc.c b/res/res_odbc.c
index 783424af664ca664f4ac8dc092b080ba3db5cf51..ea44e6f1bcd77937d4b22c8db2b2a417afc157e0 100644 (file)
--- a/res/res_odbc.c
+++ b/res/res_odbc.c
@@ -73,6 +73,7 @@ struct odbc_class
        unsigned int delme:1;                /*!< Purge the class */
        unsigned int backslash_is_escape:1;  /*!< On this database, the backslash is a native escape sequence */
        unsigned int forcecommit:1;          /*!< Should uncommitted transactions be auto-committed on handle release? */
+       unsigned int cache_is_queue:1;       /*!< Connection cache should be a queue (round-robin use) rather than a stack (last release, first re-use) */
        unsigned int isolation;              /*!< Flags for how the DB should deal with data in other, uncommitted transactions */
        unsigned int conntimeout;            /*!< Maximum time the connection process should take */
        unsigned int maxconnections;         /*!< Maximum number of allowed connections */
@@ -561,7 +562,7 @@ static int load_odbc_config(void)
        const char *dsn, *username, *password, *sanitysql;
        int enabled, bse, conntimeout, forcecommit, isolation, maxconnections, logging, slowquerylimit;
        struct timeval ncache = { 0, 0 };
-       int preconnect = 0, res = 0;
+       int preconnect = 0, res = 0, cache_is_queue = 0;
        struct ast_flags config_flags = { 0 };
 
        struct odbc_class *new;
@@ -589,6 +590,7 @@ static int load_odbc_config(void)
                        maxconnections = 1;
                        logging = 0;
                        slowquerylimit = 5000;
+                       cache_is_queue = 0;
                        for (v = ast_variable_browse(config, cat); v; v = v->next) {
                                if (!strcasecmp(v->name, "pooling") ||
                                                !strncasecmp(v->name, "share", 5) ||
@@ -644,6 +646,10 @@ static int load_odbc_config(void)
                                                ast_log(LOG_WARNING, "slow_query_limit must be a positive integer\n");
                                                slowquerylimit = 5000;
                                        }
+                               } else if (!strcasecmp(v->name, "cache_type")) {
+                                       cache_is_queue = !strcasecmp(v->value, "rr") ||
+                                               !strcasecmp(v->value, "roundrobin") ||
+                                               !strcasecmp(v->value, "queue");
                                }
                        }
 
@@ -672,6 +678,7 @@ static int load_odbc_config(void)
                                new->maxconnections = maxconnections;
                                new->logging = logging;
                                new->slowquerylimit = slowquerylimit;
+                               new->cache_is_queue = cache_is_queue;
 
                                if (cat)
                                        ast_copy_string(new->name, cat, sizeof(new->name));
@@ -758,6 +765,7 @@ static char *handle_cli_odbc_show(struct ast_cli_entry *e, int cmd, struct ast_c
                        }
 
                        ast_cli(a->fd, "    Number of active connections: %zd (out of %d)\n", class->connection_cnt, class->maxconnections);
+                       ast_cli(a->fd, "    Cache Type: %s\n", class->cache_is_queue ? "round-robin queue" : "stack (last release, first re-use)");
                        ast_cli(a->fd, "    Logging: %s\n", class->logging ? "Enabled" : "Disabled");
                        if (class->logging) {
                                ast_cli(a->fd, "    Number of prepares executed: %d\n", class->prepares_executed);
@@ -823,7 +831,11 @@ void ast_odbc_release_obj(struct odbc_obj *obj)
        obj->sql_text = NULL;
 
        ast_mutex_lock(&class->lock);
-       AST_LIST_INSERT_HEAD(&class->connections, obj, list);
+       if (class->cache_is_queue) {
+               AST_LIST_INSERT_TAIL(&class->connections, obj, list);
+       } else {
+               AST_LIST_INSERT_HEAD(&class->connections, obj, list);
+       }
        ast_cond_signal(&class->cond);
        ast_mutex_unlock(&class->lock);