[S] Change in osmo-msc[master]: sqlite optimisation: Avoid unneeded database operation - gerrit-log

31 Oct 2025

keith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28340?usp=email )
(
7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
 )Change subject: sqlite optimisation: Avoid unneeded database operation
......................................................................
sqlite optimisation: Avoid unneeded database operation
It is pointless to mark an SMS as sent immediatly
before deleting it. Let's avoid the extra database
overhead involved in doing this operation.
We change the db function which is used to delete
SMS, changing the where clause, so let's also
remove the word "sent" in the function name,
now: db_sms_delete_message_by_id()
This function is only called from one place; when
deleting sent messages, but better the name
reflects what it actually does. By the same logic,
the database statement is now called "DEL_BY_ID" and
ends up being a duplicate of what was denoted as
DEL_EXPIRED, so remove this later, which in itself has
nothing to do with the validity or expiration, but was
called from delete_expired_sms(). Call the DEL_BY_ID
statement there instead, which is a more accurate descrption
of the actual statement being run.
The unit test is changed now that we cannot
delete based on sent status anymore.
Change-Id: I777155c0f818b979c636bb59953719e472771603
---
M include/osmocom/msc/db.h
M src/libmsc/db.c
M src/libmsc/gsm_04_11.c
M src/libmsc/sms_queue.c
M tests/db_sms/db_sms_test.c
M tests/db_sms/db_sms_test.ok
6 files changed, 12 insertions(+), 24 deletions(-)
Approvals:
  Jenkins Builder: Verified
  fixeria: Looks good to me, but someone else must approve
  osmith: Looks good to me, approved

diff --git a/include/osmocom/msc/db.h b/include/osmocom/msc/db.h
index fc1781b..61c46ee 100644
--- a/include/osmocom/msc/db.h
+++ b/include/osmocom/msc/db.h
@@ -49,7 +49,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_sent_message_by_id(unsigned long long sms_id);
+int db_sms_delete_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/src/libmsc/db.c b/src/libmsc/db.c
index 43454d6..de008ad 100644
--- a/src/libmsc/db.c
+++ b/src/libmsc/db.c
@@ -53,7 +53,6 @@
    DB_STMT_SMS_INC_DELIVER_ATTEMPTS,
    DB_STMT_SMS_DEL_BY_MSISDN,
    DB_STMT_SMS_DEL_BY_ID,
-	DB_STMT_SMS_DEL_EXPIRED,
    DB_STMT_SMS_GET_VALID_UNTIL_BY_ID,
    DB_STMT_SMS_GET_OLDEST_EXPIRED,
    _NUM_DB_STMT
@@ -299,8 +298,6 @@
    [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] =
-		"DELETE FROM SMS WHERE id = $id AND sent is NOT NULL",
-	[DB_STMT_SMS_DEL_EXPIRED] =
    	"DELETE FROM SMS WHERE id = $id",
    [DB_STMT_SMS_GET_VALID_UNTIL_BY_ID] =
    	"SELECT strftime('%s', valid_until) FROM SMS WHERE id = $id",
@@ -966,7 +963,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];
@@ -988,7 +985,7 @@
 static int delete_expired_sms(unsigned long long sms_id, time_t validity_timestamp)
 {
    OSMO_ASSERT(g_dbc);
-	sqlite3_stmt *stmt = g_dbc->stmt[DB_STMT_SMS_DEL_EXPIRED];
+	sqlite3_stmt *stmt = g_dbc->stmt[DB_STMT_SMS_DEL_BY_ID];
    time_t now;
    int rc;
diff --git a/src/libmsc/gsm_04_11.c b/src/libmsc/gsm_04_11.c
index fad2f4b..530ef17 100644
--- a/src/libmsc/gsm_04_11.c
+++ b/src/libmsc/gsm_04_11.c
@@ -888,9 +888,6 @@
    				    GSM411_RP_CAUSE_PROTOCOL_ERR);
    }
-	/* mark this SMS as sent in database */
-	db_sms_mark_delivered(sms);
-
    send_signal(S_SMS_DELIVERED, trans, sms, 0);
if (sms->status_rep_req)
diff --git a/src/libmsc/sms_queue.c b/src/libmsc/sms_queue.c
index 4ef4601..8b53725 100644
--- a/src/libmsc/sms_queue.c
+++ b/src/libmsc/sms_queue.c
@@ -615,7 +615,9 @@
    	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);
+		else
+			db_sms_mark_delivered(sig_sms->sms);
    	sms_pending_free(smq, pending);
    	/* Attempt to send another SMS to this subscriber */
    	sms_send_next(vsub);
diff --git a/tests/db_sms/db_sms_test.c b/tests/db_sms/db_sms_test.c
index 352ec01..5a4ad13 100644
--- a/tests/db_sms/db_sms_test.c
+++ b/tests/db_sms/db_sms_test.c
@@ -451,22 +451,15 @@
 {
    int rc;
-	LOGP(DDB, LOGL_INFO, "Testing db_sms_delete_sent_message_by_id()...\n");
+	LOGP(DDB, LOGL_INFO, "Testing db_sms_delete_message_by_id()...\n");
-	/* Delete #1, which is marked as sent */
-	LOGP(DDB, LOGL_NOTICE, "db_sms_delete_sent_message_by_id(#1, sent): ");
-	rc = db_sms_delete_sent_message_by_id(1);
+	/* Delete #1, - (we cannot delete based on sent status) */
+	LOGP(DDB, LOGL_NOTICE, "db_sms_delete_message_by_id(#1): ");
+	rc = db_sms_delete_message_by_id(1);
    LOGPC(DDB, LOGL_NOTICE, "rc=%d\n", rc);
    /* Don't expect to retrieve this message anymore */
    sms_test_set[0].exp_db_sms_get_fail = true;
-	/* Try to delete #3, which is not marked as sent */
-	LOGP(DDB, LOGL_NOTICE, "db_sms_delete_sent_message_by_id(#3, not sent): ");
-	rc = db_sms_delete_sent_message_by_id(3);
-	LOGPC(DDB, LOGL_NOTICE, "rc=%d\n", rc);
-	/* Do expect to retrieve this message anyway */
-	sms_test_set[2].exp_db_sms_get_fail = false;
-
    LOGP(DDB, LOGL_INFO, "Testing db_sms_delete_by_msisdn()...\n");
LOGP(DDB, LOGL_NOTICE, "db_sms_delete_by_msisdn('72631'): ");
diff --git a/tests/db_sms/db_sms_test.ok b/tests/db_sms/db_sms_test.ok
index 486dcc3..03e5b11 100644
--- a/tests/db_sms/db_sms_test.ok
+++ b/tests/db_sms/db_sms_test.ok
@@ -44,9 +44,8 @@
 DDB DEBUG Marking #2 as delivered: rc=0
 DDB NOTICE db_sms_get_next_unsent(starting from #1): found
 DDB NOTICE verify_sms('Complete TP-UD (160 septets, 7-bit encoding)'): TP-User-Data mismatch
-DDB INFO Testing db_sms_delete_sent_message_by_id()...
-DDB NOTICE db_sms_delete_sent_message_by_id(#1, sent): rc=0
-DDB NOTICE db_sms_delete_sent_message_by_id(#3, not sent): rc=0
+DDB INFO Testing db_sms_delete_message_by_id()...
+DDB NOTICE db_sms_delete_message_by_id(#1): rc=0
 DDB INFO Testing db_sms_delete_by_msisdn()...
 DDB NOTICE db_sms_delete_by_msisdn('72631'): rc=0
 DDB INFO Testing db_sms_delete_oldest_expired_message()...
-- 
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28340?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I777155c0f818b979c636bb59953719e472771603
Gerrit-Change-Number: 28340
Gerrit-PatchSet: 8
Gerrit-Owner: keith keith@rhizomatica.org
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria vyanitskiy@sysmocom.de
Gerrit-Reviewer: keith keith@rhizomatica.org
Gerrit-Reviewer: osmith osmith@sysmocom.de
Gerrit-CC: pespin pespin@sysmocom.de



    