[PATCH] dbpool support for sdb

Guillaume Cottenceau <[email protected]>
Newsgroups gmane.comp.mobile.kannel.devel
Message-ID <[email protected]> 
Hi,

I have written the dbpool support for the sdb backend. Actually,
the diff is ridiculously small.


On a p4-3GHz hyperthreading, with 9 fake SMSC's and 30% of
buffered messages (testing buffered messages in a fake SMSC needs
a patch that has been posted here some time ago, but hasn't been
commented so far), performance between 1 connection and 10
connections in the dbpool is increased by 40%, which is fairly
decent. I'd expect much more on a real multi processor machine,
and/or with disks supporting real multiple writes at once (if
that exists). I have also plotted the performance when not using
DLR's, for comparison purposes[1].

http://www.mnc.ch/~gc/gatewayperf-p4-connectionpools.png

Please, Alexander or others: this patch, for the time being, is
probably really useful to others, all people who use Kannel with
DLR's and SDB. Please take one moment to look at it and possibly
integrate. It's small and non-intrusive. Implementation mimics
mysql's.

Thanks.

Ref: 
[1] details on the testing infrastructure can be found under
    "5. Performance" in one of my previous posts:

    http://www.mail-archive.com/[email protected]/msg03045.html

-- 
Guillaume Cottenceau
dbpool-for-sdb.diff (text/x-patch, 6.7 KB) 
Index: gw/dlr_sdb.c
===================================================================
RCS file: /home/cvs/gateway/gw/dlr_sdb.c,v
retrieving revision 1.7
diff -u -r1.7 dlr_sdb.c
--- gw/dlr_sdb.c	22 Jan 2004 14:08:24 -0000	1.7
+++ gw/dlr_sdb.c	17 May 2004 09:29:51 -0000
@@ -63,24 +63,25 @@
  * Andreas Fink <[email protected]>, 18.08.2001
  * Stipe Tolj <[email protected]>, 22.03.2002
  * Alexander Malysh <[email protected]> 2003
+ * Guillaume Cottenceau 2004 (dbpool support)
 */
 
 #include "gwlib/gwlib.h"
+#include "gwlib/dbpool.h"
 #include "dlr_p.h"
 
 #ifdef HAVE_SDB
 #include <sdb.h>
-static char *connection;
 
 /*
- * Database fields, which we use.
+ * Our connection pool to sdb.
  */
-static struct dlr_db_fields *fields = NULL;
+static DBPool *pool = NULL;
 
 /*
- * Mutex to protec access to database.
+ * Database fields, which we use.
  */
-static Mutex *dlr_mutex = NULL;
+static struct dlr_db_fields *fields = NULL;
 
 enum {
     SDB_ORACLE,
@@ -106,9 +107,27 @@
 
 static void dlr_sdb_shutdown()
 {
-    sdb_close(connection);
+    dbpool_destroy(pool);
     dlr_db_fields_destroy(fields);
-    mutex_destroy(dlr_mutex);
+}
+
+static int gw_sdb_query(char *query,
+                        int (*callback)(int, char **, void *), void *closure)
+{
+    DBPoolConn *pc;
+    int rows;
+
+    pc = dbpool_conn_consume(pool);
+    if (pc == NULL) {
+        error(0, "SDB: Database pool got no connection!");
+        return -1;
+    }
+
+    rows = sdb_query(pc->conn, query, callback, closure);
+
+    dbpool_conn_produce(pc);
+
+    return rows;
 }
 
 static void dlr_sdb_add(struct dlr_entry *dlr)
@@ -133,9 +152,7 @@
      debug("dlr.sdb", 0, "SDB: sql: %s", octstr_get_cstr(sql));
 #endif
 
-    mutex_lock(dlr_mutex);
-    state = sdb_query(connection, octstr_get_cstr(sql), NULL, NULL);
-    mutex_unlock(dlr_mutex);
+    state = gw_sdb_query(octstr_get_cstr(sql), NULL, NULL);
     if (state == -1)
         error(0, "SDB: error in inserting DLR for DST <%s>", octstr_get_cstr(dlr->destination));
 
@@ -209,9 +226,7 @@
      debug("dlr.sdb", 0, "SDB: sql: %s", octstr_get_cstr(sql));
 #endif
 
-    mutex_lock(dlr_mutex);
-    state = sdb_query(connection, octstr_get_cstr(sql), sdb_callback_add, res);
-    mutex_unlock(dlr_mutex);
+    state = gw_sdb_query(octstr_get_cstr(sql), sdb_callback_add, res);
     octstr_destroy(sql);
     if (state == -1) {
         error(0, "SDB: error in finding DLR");
@@ -247,9 +262,7 @@
      debug("dlr.sdb", 0, "SDB: sql: %s", octstr_get_cstr(sql));
 #endif
 
-    mutex_lock(dlr_mutex);
-    state = sdb_query(connection, octstr_get_cstr(sql), NULL, NULL);
-    mutex_unlock(dlr_mutex);
+    state = gw_sdb_query(octstr_get_cstr(sql), NULL, NULL);
     octstr_destroy(sql);
     if (state == -1) {
         error(0, "SDB: error in updating DLR");
@@ -271,9 +284,7 @@
      debug("dlr.sdb", 0, "SDB: sql: %s", octstr_get_cstr(sql));
 #endif
 
-    mutex_lock(dlr_mutex);
-    state = sdb_query(connection, octstr_get_cstr(sql), NULL, NULL);
-    mutex_unlock(dlr_mutex);
+    state = gw_sdb_query(octstr_get_cstr(sql), NULL, NULL);
     octstr_destroy(sql);
     if (state == -1)
         error(0, "SDB: error in deleting DLR");
@@ -291,13 +302,10 @@
     debug("dlr.sdb", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
 
-    mutex_lock(dlr_mutex);
-    state = sdb_query(connection, octstr_get_cstr(sql), sdb_callback_msgs, &res);
-    mutex_unlock(dlr_mutex);
+    state = gw_sdb_query(octstr_get_cstr(sql), sdb_callback_msgs, &res);
     octstr_destroy(sql);
     if (state == -1) {
         error(0, "SDB: error in selecting ammount of waiting DLRs");
-        mutex_unlock(dlr_mutex);
         return -1;
     }
 
@@ -315,9 +323,7 @@
      debug("dlr.sdb", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
 
-    mutex_lock(dlr_mutex);
-    state = sdb_query(connection, octstr_get_cstr(sql), NULL, NULL);
-    mutex_unlock(dlr_mutex);
+    state = gw_sdb_query(octstr_get_cstr(sql), NULL, NULL);
     octstr_destroy(sql);
     if (state == -1) {
         error(0, "SDB: error in flusing DLR table");
@@ -342,6 +348,8 @@
     List *grplist;
     Octstr *sdb_url, *sdb_id;
     Octstr *p = NULL;
+    long pool_size;
+    DBConf *db_conf = NULL;
 
     /*
      * check for all mandatory directives that specify the field names
@@ -355,7 +363,6 @@
 
     fields = dlr_db_fields_create(grp);
     gw_assert(fields != NULL);
-    dlr_mutex = mutex_create();
 
     /*
      * now grap the required information from the 'mysql-connection' group
@@ -380,6 +387,9 @@
     octstr_destroy(p);
     list_destroy(grplist, NULL);
 
+    if (cfg_get_integer(&pool_size, grp, octstr_imm("max-connections")) == -1 || pool_size == 0)
+        pool_size = 1;
+
     if (!(sdb_url = cfg_get(grp, octstr_imm("url"))))
    	    panic(0, "DLR: SDB: directive 'url' is not specified!");
 
@@ -396,11 +406,23 @@
      * ok, ready to connect
      */
     info(0,"Connecting to sdb resource <%s>.", octstr_get_cstr(sdb_url));
-    connection = sdb_open(octstr_get_cstr(sdb_url));
-    if (connection == NULL)
-        panic(0, "Could not connect to database");
 
-    octstr_destroy(sdb_url);
+    db_conf = gw_malloc(sizeof(DBConf));
+    gw_assert(db_conf != NULL);
+
+    db_conf->sdb = gw_malloc(sizeof(SDBConf));
+    gw_assert(db_conf->sdb != NULL);
+
+    db_conf->sdb->url = sdb_url;
+
+    pool = dbpool_create(DBPOOL_SDB, db_conf, pool_size);
+    gw_assert(pool != NULL);
+
+    /*
+     * XXX should a failing connect throw panic?!
+     */
+    if (dbpool_conn_count(pool) == 0)
+        panic(0,"DLR: SDB: database pool has no connections!");
 
     return &handles;
 }
Index: gwlib/cfg.def
===================================================================
RCS file: /home/cvs/gateway/gwlib/cfg.def,v
retrieving revision 1.103
diff -u -r1.103 cfg.def
--- gwlib/cfg.def	19 Apr 2004 19:08:56 -0000	1.103
+++ gwlib/cfg.def	17 May 2004 09:29:51 -0000
@@ -501,6 +501,7 @@
 MULTI_GROUP(sdb-connection,
     OCTSTR(id)
     OCTSTR(url)
+    OCTSTR(max-connections)
 )
 
  
Index: gwlib/dbpool.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/dbpool.c,v
retrieving revision 1.9
diff -u -r1.9 dbpool.c
--- gwlib/dbpool.c	20 Mar 2004 19:48:22 -0000	1.9
+++ gwlib/dbpool.c	17 May 2004 09:29:51 -0000
@@ -76,6 +76,7 @@
 #include "dbpool_mysql.c"
 #include "dbpool_oracle.c"
 #include "dbpool_sqlite.c"
+#include "dbpool_sdb.c"
 
 
 static inline void dbpool_conn_destroy(DBPoolConn *conn)
@@ -124,8 +125,11 @@
             p->db_ops = &sqlite_ops;
             break;
 #endif
+#ifdef HAVE_SDB
         case DBPOOL_SDB:
-            panic(0, "DBPOOL for libsdb not yet implemented");
+            p->db_ops = &sdb_ops;
+            break;
+#endif
         default:
             panic(0, "Unknown dbpool type defined.");
     }
dbpool_sdb.c (application/octet-stream, 3.9 KB) 
/* ==================================================================== 
 * The Kannel Software License, Version 1.0 
 * 
 * Copyright (c) 2001-2004 Kannel Group  
 * Copyright (c) 1998-2001 WapIT Ltd.   
 * All rights reserved. 
 * 
 * Redistribution and use in source and binary forms, with or without 
 * modification, are permitted provided that the following conditions 
 * are met: 
 * 
 * 1. Redistributions of source code must retain the above copyright 
 *    notice, this list of conditions and the following disclaimer. 
 * 
 * 2. Redistributions in binary form must reproduce the above copyright 
 *    notice, this list of conditions and the following disclaimer in 
 *    the documentation and/or other materials provided with the 
 *    distribution. 
 * 
 * 3. The end-user documentation included with the redistribution, 
 *    if any, must include the following acknowledgment: 
 *       "This product includes software developed by the 
 *        Kannel Group (http://www.kannel.org/)." 
 *    Alternately, this acknowledgment may appear in the software itself, 
 *    if and wherever such third-party acknowledgments normally appear. 
 * 
 * 4. The names "Kannel" and "Kannel Group" must not be used to 
 *    endorse or promote products derived from this software without 
 *    prior written permission. For written permission, please  
 *    contact [email protected]. 
 * 
 * 5. Products derived from this software may not be called "Kannel", 
 *    nor may "Kannel" appear in their name, without prior written 
 *    permission of the Kannel Group. 
 * 
 * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED 
 * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES 
 * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE 
 * DISCLAIMED.  IN NO EVENT SHALL THE KANNEL GROUP OR ITS CONTRIBUTORS 
 * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,  
 * OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT  
 * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR  
 * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,  
 * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE  
 * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,  
 * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
 * ==================================================================== 
 * 
 * This software consists of voluntary contributions made by many 
 * individuals on behalf of the Kannel Group.  For more information on  
 * the Kannel Group, please see <http://www.kannel.org/>. 
 * 
 * Portions of this software are based upon software originally written at  
 * WapIT Ltd., Helsinki, Finland for the Kannel project.  
 */ 

/*
 * dbpool_sdbl.c - implement SDB operations for generic database connection pool
 *
 * Guillaume Cottenceau, 2004
 */

#ifdef HAVE_SDB
#include <sdb.h>

static void* sdb_open_conn(const DBConf *db_conf)
{
    SDBConf *conf = db_conf->sdb; /* make compiler happy */
    char *connection;

    /* sanity check */
    if (conf == NULL)
        return NULL;

    connection = sdb_open(octstr_get_cstr(conf->url));
    if (connection == NULL) {
        error(0, "SDB: could not connect to database");
        return NULL;
    }

    info(0, "SDB: Connected to %s.", octstr_get_cstr(conf->url));

    return connection;
}

static void sdb_close_conn(void *conn)
{
    if (conn == NULL)
        return;

    sdb_close(conn);
    gw_free(conn);
}

static int sdb_check_conn(void *conn)
{
    if (conn == NULL)
        return -1;

    /* nothing in SDB to check for the connection, always succeed */
    return 0;
}

static void sdb_conf_destroy(DBConf *db_conf)
{
    SDBConf *conf = db_conf->sdb;

    octstr_destroy(conf->url);

    gw_free(conf);
    gw_free(db_conf);
}

static struct db_ops sdb_ops = {
    .open = sdb_open_conn,
    .close = sdb_close_conn,
    .check = sdb_check_conn,
    .conf_destroy = sdb_conf_destroy
};

#endif /* HAVE_SDB */
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.