Re: dlr_mysql.c patch

"Nikos Balkanas" <[email protected]> Thu, 22 Oct 2009 14:39:00 +0300

Newsgroups	gmane.comp.mobile.kannel.devel
Message-ID	<001801ca530c$43c24ac0$02b2a8c0@tardis>

Here you go.

BR,
Nikos
----- Original Message ----- 
From: "Alexander Malysh" <[email protected]>
To: "Nikos Balkanas" <[email protected]>
Cc: <[email protected]>
Sent: Thursday, October 22, 2009 11:06 AM
Subject: Re: dlr_mysql.c patch


Hi Niko,

could you please send this as one patch because it's too difficult to
review?

Thanks,
Alexander Malysh

Am 22.10.2009 um 01:30 schrieb Nikos Balkanas:

> 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><dlr_mysql.diff><dlr_oracle.diff><dlr_pgsql.diff><dlr_sdb.diff>

kannel.diff (application/octet-stream, 8.7 KB)

--- gw/dlr_sdb.c	2009-10-22 14:27:23.000000000 +0300
+++ gw/dlr_sdb.c	2009-10-22 02:04:48.000000000 +0300
@@ -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)
--- gw/dlr_mysql.c	2009-10-22 14:27:23.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);
--- gw/dlr_mssql.c	2009-10-22 14:27:23.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);
--- gw/dlr_oracle.c	2009-10-22 14:27:23.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);
--- gw/dlr_pgsql.c	2009-10-22 14:27:23.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);
 }