Re: dlr_mysql.c patch

"Nikos Balkanas" <[email protected]> Thu, 22 Oct 2009 02:30:13 +0300

Newsgroups	gmane.comp.mobile.kannel.devel
Message-ID	<001e01ca52a6$718ec070$02b2a8c0@tardis>

Here you go. Sorry for the delay, got caught up with my work. I expanded to 
rest of dbs and to insert and delete statements.

BR,
Nikos
----- Original Message ----- 
From: "Alexander Malysh" <[email protected]>
To: "Nikos Balkanas" <[email protected]>
Cc: <[email protected]>
Sent: Wednesday, October 14, 2009 10:25 AM
Subject: Re: dlr_mysql.c patch


Hi,

I don't think we should mark this as error, warning should be
sufficient, and please fix indentation:
if (...)
     warning

auto-reconnect is enabled for mysql as default option (AFAIK). We
trying to reconnect because if
mysql server is not available for say 2 minutes (e.g. restart of mysql
daemon) then mysql reconnect feature
doesn't work but our reconnect will work.

Thanks,
Alexander Malysh

Am 14.10.2009 um 01:28 schrieb Nikos Balkanas:

> Gladly,
>
> It is something I found along the way and thought it might be useful  to 
> others.  No biggie, though.
>
> We are returning error -1 from the database when connection or other 
> problem prevents the SQL from executing. In the same spirit I feel  it is 
> appropriate to return error from the database when we try an  update limit 
> 1 and 0 rows are affected.
>
> In a operational environment DLR mismatch could happen due to a lot  of 
> causes, some of them unrelated to the database. This provides for  cleaner 
> view of the source of the problem, than mismatch could  indicate.
>
> In our case we mismatch ~2% of the DLRs. We use a single SMSc,  SMPPbox, 
> and use the default (literal) msg-id-type for that. In a  test run of 
> 50000 MTs to FAKE smsc, only 20 DLRs were mismatched,  indicating that 
> with real SMScs, insertion speed is not a factor.  From time 2 time we 
> delete leftover DLRs from the database. Then an  EXPIRED DLR arrives and 
> naturally is mismatched.
>
> If you see the logic in this, I could apply it to the rest of the 
> databases.
>
> BTW, I noticed that in dbpool-mysql.c if mysql_ping fails we  manually 
> reconnect. Isn't it about time to use the auto-reconnect  option for 
> mysql?
>
> BR,
> Nikos
>
> ----- Original Message ----- From: "Stipe Tolj" <[email protected]>
> Cc: <[email protected]>
> Sent: Wednesday, October 14, 2009 1:27 AM
> Subject: Re: dlr_mysql.c patch
>
>
>> Nikos Balkanas schrieb:
>>> Hi,
>>>
>>> A simple patch to help with missing dlrs when using mysql.
>>
>> Hi Nikos,
>>
>> I'm -0 for this patch. Can you comment on why it's needed and  what's the
>> intention behind it?... in other words: try to sell it to us :)
>>
>> Stipe
>>
>> -- 
>> -------------------------------------------------------------------
>> KΞ“Β¶lner Landstrasse 419
>> 40589 DΞ“Ξsseldorf, NRW, Germany
>>
>> tolj.org system architecture      Kannel Software Foundation (KSF)
>> http://www.tolj.org/              http://www.kannel.org/
>>
>> mailto:st_{at}_tolj.org           mailto:stolj_{at}_kannel.org
>> -------------------------------------------------------------------
>
>

dlr_mssql.diff (application/octet-stream, 2.1 KB)

--- gw/dlr_mssql.c	2009-10-22 02:10:51.000000000 +0300
+++ gw/dlr_mssql.c	2009-10-22 01:48:24.000000000 +0300
@@ -129,6 +129,7 @@
     Octstr *sql;
     DBPoolConn *pconn;
     debug("dlr.mssql", 0, "adding DLR entry into database");
+    int res;
 
     pconn = dbpool_conn_consume(pool);
     /* just for sure */
@@ -148,8 +149,10 @@
 #if defined(DLR_TRACE)
     debug("dlr.mssql", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
-    if (dbpool_conn_update(pconn, sql, NULL) == -1)
+    if ((res = dbpool_conn_update(pconn, sql, NULL)) == -1)
         error(0, "DLR: MSSQL: Error while adding dlr entry for DST<%s>", octstr_get_cstr(entry->destination));
+    else if (!res)
+        warning(0, "DLR: MSSQL: No dlr inserted for DST<%s>", octstr_get_cstr(entry->destination));
 
     dbpool_conn_produce(pconn);
     octstr_destroy(sql);
@@ -160,6 +163,7 @@
 {
     Octstr *sql;
     DBPoolConn *pconn;
+    int res;
 
     debug("dlr.mssql", 0, "removing DLR from database");
 
@@ -178,8 +182,10 @@
     debug("dlr.mssql", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
 
-    if (dbpool_conn_update(pconn, sql, NULL) == -1)
+    if ((res = dbpool_conn_update(pconn, sql, NULL)) == -1)
         error(0, "DLR: MSSQL: Error while removing dlr entry for DST<%s>", octstr_get_cstr(dst));
+    else if (!res)
+        warning(0, "DLR: MSSQL: No dlr deleted for DST<%s>", octstr_get_cstr(dst));
 
     dbpool_conn_produce(pconn);
     octstr_destroy(sql);
@@ -238,6 +244,7 @@
 {
     Octstr *sql, *os_status;
     DBPoolConn *pconn;
+    int res;
 
     debug("dlr.mssql", 0, "updating DLR status in database");
 
@@ -255,8 +262,10 @@
 #if defined(DLR_TRACE)
     debug("dlr.mssql", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
-    if (dbpool_conn_update(pconn, sql, NULL) == -1)
+    if ((res = dbpool_conn_update(pconn, sql, NULL)) == -1)
         error(0, "DLR: MSSQL: Error while updating dlr entry for DST<%s>", octstr_get_cstr(dst));
+    else if (!res)
+        warning(0, "DLR: MSSQL: No dlr found to update for DST<%s> (status: %d)", octstr_get_cstr(dst), status);
 
     dbpool_conn_produce(pconn);
     octstr_destroy(os_status);

dlr_mysql.diff (application/octet-stream, 2.2 KB)

--- gw/dlr_mysql.c	2009-10-22 02:21:26.000000000 +0300
+++ gw/dlr_mysql.c	2009-10-22 02:18:28.000000000 +0300
@@ -94,6 +94,8 @@
     Octstr *sql, *os_mask;
     DBPoolConn *pconn;
     List *binds = gwlist_create();
+    int res;
+
     debug("dlr.mysql", 0, "adding DLR entry into database");
 
     pconn = dbpool_conn_consume(pool);
@@ -122,8 +124,10 @@
 #if defined(DLR_TRACE)
     debug("dlr.mysql", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
-    if (dbpool_conn_update(pconn, sql, binds) == -1)
+    if ((res = dbpool_conn_update(pconn, sql, binds)) == -1)
         error(0, "DLR: MYSQL: Error while adding dlr entry for DST<%s>", octstr_get_cstr(entry->destination));
+    else if (!res)
+        warning(0, "DLR: MYSQL: No dlr inserted for DST<%s>", octstr_get_cstr(entry->destination));
 
     dbpool_conn_produce(pconn);
     octstr_destroy(sql);
@@ -194,6 +198,7 @@
     Octstr *sql;
     DBPoolConn *pconn;
     List *binds = gwlist_create();
+    int res;
 
     debug("dlr.mysql", 0, "removing DLR from database");
 
@@ -213,8 +218,10 @@
     debug("dlr.mysql", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
 
-    if (dbpool_conn_update(pconn, sql, binds) == -1)
+    if ((res = dbpool_conn_update(pconn, sql, binds)) == -1)
         error(0, "DLR: MYSQL: Error while removing dlr entry for DST<%s>", octstr_get_cstr(dst));
+    else if (!res)
+        warning(0, "DLR: MYSQL: No dlr deleted for DST<%s>", octstr_get_cstr(dst));
 
     dbpool_conn_produce(pconn);
     gwlist_destroy(binds, NULL);
@@ -226,6 +233,7 @@
     Octstr *sql, *os_status;
     DBPoolConn *pconn;
     List *binds = gwlist_create();
+    int res;
 
     debug("dlr.mysql", 0, "updating DLR status in database");
 
@@ -246,8 +254,10 @@
 #if defined(DLR_TRACE)
     debug("dlr.mysql", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
-    if (dbpool_conn_update(pconn, sql, binds) == -1)
+    if ((res = dbpool_conn_update(pconn, sql, binds)) == -1)
         error(0, "DLR: MYSQL: Error while updating dlr entry for DST<%s>", octstr_get_cstr(dst));
+    else if (!res)
+       warning(0, "DLR: MYSQL: No dlr found to update for DST<%s>, (status %d)", octstr_get_cstr(dst), status);
 
     dbpool_conn_produce(pconn);
     gwlist_destroy(binds, NULL);

dlr_oracle.diff (application/octet-stream, 2.2 KB)

--- gw/dlr_oracle.c	2009-10-22 02:10:51.000000000 +0300
+++ gw/dlr_oracle.c	2009-10-22 01:49:11.000000000 +0300
@@ -127,6 +127,8 @@
     Octstr *sql, *os_mask;
     DBPoolConn *pconn;
     List *binds = gwlist_create();
+    int res;
+
     debug("dlr.oracle", 0, "adding DLR entry into database");
 
     pconn = dbpool_conn_consume(pool);
@@ -155,8 +157,10 @@
 #if defined(DLR_TRACE)
     debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
-    if (dbpool_conn_update(pconn, sql, binds) == -1)
+    if ((res = dbpool_conn_update(pconn, sql, binds)) == -1)
         error(0, "DLR: ORACLE: Error while adding dlr entry for DST<%s>", octstr_get_cstr(entry->destination));
+    else if (!res)
+        warning(0, "DLR: ORACLE: No dlr inserted for DST<%s>", octstr_get_cstr(entry->destination));
 
     dbpool_conn_produce(pconn);
     octstr_destroy(sql);
@@ -170,6 +174,7 @@
     Octstr *sql;
     DBPoolConn *pconn;
     List *binds = gwlist_create();
+    int res;
 
     debug("dlr.oracle", 0, "removing DLR from database");
 
@@ -190,8 +195,10 @@
     debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
 
-    if (dbpool_conn_update(pconn, sql, binds) == -1)
+    if ((res = dbpool_conn_update(pconn, sql, binds)) == -1)
         error(0, "DLR: ORACLE: Error while removing dlr entry for DST<%s>", octstr_get_cstr(dst));
+    else if (!res)
+        warning(0, "DLR: ORACLE: No dlr deleted for DST<%s>", octstr_get_cstr(dst));
 
     dbpool_conn_produce(pconn);
     gwlist_destroy(binds, NULL);
@@ -260,6 +267,7 @@
     Octstr *sql, *os_status;
     DBPoolConn *pconn;
     List *binds = gwlist_create();
+    int res;
 
     debug("dlr.oracle", 0, "updating DLR status in database");
 
@@ -280,8 +288,10 @@
 #if defined(DLR_TRACE)
     debug("dlr.oracle", 0, "sql: %s", octstr_get_cstr(sql));
 #endif
-    if (dbpool_conn_update(pconn, sql, binds) == -1)
+    if ((res = dbpool_conn_update(pconn, sql, binds)) == -1)
         error(0, "DLR: ORACLE: Error while updating dlr entry for DST<%s>", octstr_get_cstr(dst));
+    else if (!res)
+        warning(0, "DLR: ORACLE: No dlr found to update for DST<%s> (status: %d)", octstr_get_cstr(dst), status);
 
     dbpool_conn_produce(pconn);
     gwlist_destroy(binds, NULL);

dlr_pgsql.diff (application/octet-stream, 1.1 KB)

--- gw/dlr_pgsql.c	2009-10-22 02:10:51.000000000 +0300
+++ gw/dlr_pgsql.c	2009-10-22 01:56:23.000000000 +0300
@@ -154,7 +154,8 @@
                         entry->mask, octstr_get_cstr(entry->boxc_id), 0);
 
 
-    pgsql_update(sql);
+    if (!pgsql_update(sql))
+       warning(0, "DLR: PGSQL: No dlr inserted for DST<%s>", octstr_get_cstr(entry->destination));
     
     octstr_destroy(sql);
     dlr_entry_destroy(entry);
@@ -226,7 +227,8 @@
                         octstr_get_cstr(smsc), octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
 
 
-    pgsql_update(sql);
+    if (!pgsql_update(sql))
+       warning(0, "DLR: PGSQL: No dlr deleted for DST<%s>", octstr_get_cstr(dst));
     octstr_destroy(sql);
 }
 
@@ -242,7 +244,8 @@
                         octstr_get_cstr(fields->table),
                         octstr_get_cstr(fields->field_smsc), octstr_get_cstr(smsc),
                         octstr_get_cstr(fields->field_ts), octstr_get_cstr(ts));
-    pgsql_update(sql);
+    if (!pgsql_update(sql))
+       warning(0, "DLR: PGSQL: No dlr updated for DST<%s> (status: %d)", octstr_get_cstr(dst), status);
     octstr_destroy(sql);
 }

dlr_sdb.diff (application/octet-stream, 1.3 KB)

--- gw/dlr_sdb.c	2009-10-22 02:10:51.000000000 +0300
+++ gw/dlr_sdb.c	2009-10-22 02:04:48.000000000 +0300
@@ -93,7 +93,7 @@
 static long sdb_conn_type = SDB_OTHER;
 
 
-static const char* sdb_get_limit_str()
+const char* sdb_get_limit_str()
 {
     switch (sdb_conn_type) {
         case SDB_ORACLE:
@@ -157,6 +157,8 @@
     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));
+    else if (!state)
+        warning(0, "SDB: No dlr inserted for DST <%s>", octstr_get_cstr(dlr->destination));
 
     octstr_destroy(sql);
     dlr_entry_destroy(dlr);
@@ -266,9 +268,10 @@
 
     state = gw_sdb_query(octstr_get_cstr(sql), NULL, NULL);
     octstr_destroy(sql);
-    if (state == -1) {
+    if (state == -1)
         error(0, "SDB: error in updating DLR");
-    }
+    else if (!state)
+        warning(0, "SDB: No dlr to update for DST<%s> (status %d)", octstr_get_cstr(dst), status);
 }
 
 static void  dlr_sdb_remove(const Octstr *smsc, const Octstr *ts, const Octstr *dst)
@@ -306,6 +309,8 @@
     octstr_destroy(sql);
     if (state == -1)
         error(0, "SDB: error in deleting DLR");
+    else if (!state)
+        warning(0, "SDB: No dlr deleted for DST<%s>", octstr_get_cstr(dst));
 }
 
 static long dlr_sdb_messages(void)