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

5 files changed, 54 insertions, 23 deletions

diff --git a/include/osmocom/msc/db.h b/include/osmocom/msc/db.h
index fc1781bd6..7392bd270 100644
--- a/include/osmocom/msc/db.h
+++ b/include/osmocom/msc/db.h
@@ -49,6 +49,7 @@ struct gsm_sms *db_sms_get_unsent_for_subscr(struct vlr_subscr *vsub,
 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 a6e6aebd3..e4b2a120d 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 134eb4afa..24f1d002c 100644
--- a/src/libmsc/db.c
+++ b/src/libmsc/db.c
@@ -53,6 +53,7 @@ enum stmt_idx {
 	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,
@@ -298,8 +299,10 @@ static const char *stmt_sql[] = {
 		" 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] =
@@ -974,7 +977,7 @@ int db_sms_delete_by_msisdn(const char *msisdn)
 	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];
@@ -993,6 +996,25 @@ int db_sms_delete_sent_message_by_id(unsigned long long sms_id)
 	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 aa87a192a..7e2869d50 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 <osmocom/smpp/smpp_smsc.h>
@@ -845,6 +846,7 @@ static int gsm411_rx_rp_ack(struct gsm_trans *trans,
 			    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 @@ static int gsm411_rx_rp_ack(struct gsm_trans *trans,
 					    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 c6e9a12c9..4a362db8a 100644
--- a/src/libmsc/sms_queue.c
+++ b/src/libmsc/sms_queue.c
@@ -119,23 +119,6 @@ struct gsm_sms_pending {
 	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)
 {
@@ -617,7 +600,7 @@ static int sms_sms_cb(unsigned int subsys, unsigned int signal,
 		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);