keith has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/28340 ) Change subject: Avoid UPDATE immediately followed by DELETE ...................................................................... Avoid UPDATE immediately followed by DELETE There is no need to mark an SMS as sent before deleting it. Avoid the extra database overhead involved in doing this. Change-Id: I777155c0f818b979c636bb59953719e472771603 --- M include/osmocom/msc/db.h M include/osmocom/msc/sms_queue.h M src/libmsc/db.c M src/libmsc/gsm_04_11.c M src/libmsc/sms_queue.c 5 files changed, 54 insertions(+), 23 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/40/28340/1 diff --git a/include/osmocom/msc/db.h b/include/osmocom/msc/db.h index a67c050..4294b9f 100644 --- a/include/osmocom/msc/db.h +++ b/include/osmocom/msc/db.h @@ -50,6 +50,7 @@ int db_sms_mark_delivered(struct gsm_sms *sms); int db_sms_inc_deliver_attempts(struct gsm_sms *sms); int db_sms_delete_by_msisdn(const char *msisdn); +int db_sms_delete_message_by_id(unsigned long long sms_id); int db_sms_delete_sent_message_by_id(unsigned long long sms_id); int db_sms_delete_expired_message_by_id(unsigned long long sms_id); void db_sms_delete_oldest_expired_message(void); diff --git a/include/osmocom/msc/sms_queue.h b/include/osmocom/msc/sms_queue.h index a6e6aeb..e4b2a12 100644 --- a/include/osmocom/msc/sms_queue.h +++ b/include/osmocom/msc/sms_queue.h @@ -2,9 +2,26 @@ #define SMS_QUEUE_H #include <stdbool.h> +#include <osmocom/core/timer.h> +#include <osmocom/msc/gsm_subscriber.h> struct gsm_network; -struct gsm_sms_queue; +/* (global) state of the SMS queue. */ +struct gsm_sms_queue { + struct osmo_timer_list resend_pending; /* timer triggering sms_resend_pending() */ + struct osmo_timer_list push_queue; /* timer triggering sms_submit_pending() */ + struct gsm_network *network; + struct llist_head pending_sms; /* list of gsm_sms_pending */ + struct sms_queue_config *cfg; + int pending; /* current number of gsm_sms_pending in RAM */ + + /* last MSISDN for which we read SMS from the database and created gsm_sms_pending records */ + char last_msisdn[GSM23003_MSISDN_MAX_DIGITS+1]; + + /* statistics / counters */ + struct osmo_stat_item_group *statg; + struct rate_ctr_group *ctrg; +}; struct vty; struct sms_queue_config { diff --git a/src/libmsc/db.c b/src/libmsc/db.c index 6fe8803..ea0fef3 100644 --- a/src/libmsc/db.c +++ b/src/libmsc/db.c @@ -54,6 +54,7 @@ DB_STMT_SMS_INC_DELIVER_ATTEMPTS, DB_STMT_SMS_DEL_BY_MSISDN, DB_STMT_SMS_DEL_BY_ID, + DB_STMT_SMS_DEL_SENT_BY_ID, DB_STMT_SMS_DEL_EXPIRED, DB_STMT_SMS_GET_VALID_UNTIL_BY_ID, DB_STMT_SMS_GET_OLDEST_EXPIRED, @@ -310,8 +311,10 @@ " WHERE id = $id", [DB_STMT_SMS_DEL_BY_MSISDN] = "DELETE FROM SMS WHERE src_addr=$src_addr OR dest_addr=$dest_addr", - [DB_STMT_SMS_DEL_BY_ID] = + [DB_STMT_SMS_DEL_SENT_BY_ID] = "DELETE FROM SMS WHERE id = $id AND sent is NOT NULL", + [DB_STMT_SMS_DEL_BY_ID] = + "DELETE FROM SMS WHERE id = $id LIMIT 1", [DB_STMT_SMS_DEL_EXPIRED] = "DELETE FROM SMS WHERE id = $id", [DB_STMT_SMS_GET_VALID_UNTIL_BY_ID] = @@ -986,7 +989,7 @@ return 0; } -int db_sms_delete_sent_message_by_id(unsigned long long sms_id) +int db_sms_delete_message_by_id(unsigned long long sms_id) { OSMO_ASSERT(g_dbc); sqlite3_stmt *stmt = g_dbc->stmt[DB_STMT_SMS_DEL_BY_ID]; @@ -1005,6 +1008,25 @@ return 0; } +int db_sms_delete_sent_message_by_id(unsigned long long sms_id) +{ + OSMO_ASSERT(g_dbc); + sqlite3_stmt *stmt = g_dbc->stmt[DB_STMT_SMS_DEL_SENT_BY_ID]; + int rc; + + db_bind_int64(stmt, "$id", sms_id); + + rc = sqlite3_step(stmt); + if (rc != SQLITE_DONE) { + db_remove_reset(stmt); + LOGP(DDB, LOGL_ERROR, "Failed to delete SMS %llu.\n", sms_id); + return 1; + } + + db_remove_reset(stmt); + return 0; +} + static int delete_expired_sms(unsigned long long sms_id, time_t validity_timestamp) { OSMO_ASSERT(g_dbc); diff --git a/src/libmsc/gsm_04_11.c b/src/libmsc/gsm_04_11.c index 743e272..a9d5bd6 100644 --- a/src/libmsc/gsm_04_11.c +++ b/src/libmsc/gsm_04_11.c @@ -56,6 +56,7 @@ #include <osmocom/msc/msub.h> #include <osmocom/msc/msc_a.h> #include <osmocom/msc/paging.h> +#include <osmocom/msc/sms_queue.h> #ifdef BUILD_SMPP #include "smpp_smsc.h" @@ -845,6 +846,7 @@ struct gsm411_rp_hdr *rph) { struct gsm_sms *sms = trans->sms.sms; + struct gsm_sms_queue *smq = trans->net->sms_queue; /* Acnkowledgement to MT RP_DATA, i.e. the MS confirms it * successfully received a SMS. We can now safely mark it as @@ -861,8 +863,14 @@ GSM411_RP_CAUSE_PROTOCOL_ERR); } - /* mark this SMS as sent in database */ - db_sms_mark_delivered(sms); + /* If we are deleting delivered SMS, then this SMS will very soon be deleted + * in the signal callback. + * In this case, let's avoid the extra database overhead involved in doing + * an UPDATE followed by an immediate DELETE */ + + /* msc_vlr tests will not pass any sms_queue, hence need to check smq != NULL */ + if (smq && smq->cfg->delete_delivered == 0) + db_sms_mark_delivered(sms); send_signal(S_SMS_DELIVERED, trans, sms, 0); diff --git a/src/libmsc/sms_queue.c b/src/libmsc/sms_queue.c index a1cc465..9b42024 100644 --- a/src/libmsc/sms_queue.c +++ b/src/libmsc/sms_queue.c @@ -119,23 +119,6 @@ int resend; /* should we try re-sending it (now) ? */ }; -/* (global) state of the SMS queue. */ -struct gsm_sms_queue { - struct osmo_timer_list resend_pending; /* timer triggering sms_resend_pending() */ - struct osmo_timer_list push_queue; /* timer triggering sms_submit_pending() */ - struct gsm_network *network; - struct llist_head pending_sms; /* list of gsm_sms_pending */ - struct sms_queue_config *cfg; - int pending; /* current number of gsm_sms_pending in RAM */ - - /* last MSISDN for which we read SMS from the database and created gsm_sms_pending records */ - char last_msisdn[GSM23003_MSISDN_MAX_DIGITS+1]; - - /* statistics / counters */ - struct osmo_stat_item_group *statg; - struct rate_ctr_group *ctrg; -}; - /* private wrapper function to make sure we count all SMS delivery attempts */ static void _gsm411_send_sms(struct gsm_network *net, struct vlr_subscr *vsub, struct gsm_sms *sms) { @@ -625,7 +608,7 @@ vsub = pending->vsub; vlr_subscr_get(vsub, __func__); if (smq->cfg->delete_delivered) - db_sms_delete_sent_message_by_id(pending->sms_id); + db_sms_delete_message_by_id(pending->sms_id); sms_pending_free(smq, pending); /* Attempt to send another SMS to this subscriber */ sms_send_next(vsub); -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28340 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I777155c0f818b979c636bb59953719e472771603 Gerrit-Change-Number: 28340 Gerrit-PatchSet: 1 Gerrit-Owner: keith &lt;keith(a)rhizomatica.org&gt; Gerrit-MessageType: newchange