Hi,

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

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.");
     }

Attachment: dbpool_sdb.c
Description: Binary data

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

Reply via email to