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(a)rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: keith <keith(a)rhizomatica.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Attention is currently required from: laforge, osmith.
keith has posted comments on this change by keith. ( https://gerrit.osmocom.org/c/osmo-msc/+/41221?usp=email )
Change subject: sqlite optimisation: check VLR earlier for dest. subscriber
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
No Code Change was made,so manually preserving previous 2 x +1
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/41221?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ibd07b9d200b48108d705ed4c461cc4ddfa421fd2
Gerrit-Change-Number: 41221
Gerrit-PatchSet: 3
Gerrit-Owner: keith <keith(a)rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith <keith(a)rhizomatica.org>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Fri, 31 Oct 2025 20:17:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: laforge, osmith.
keith has posted comments on this change by keith. ( https://gerrit.osmocom.org/c/osmo-msc/+/41221?usp=email )
Change subject: sqlite optimisation: check VLR earlier for dest. subscriber
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/41221?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ibd07b9d200b48108d705ed4c461cc4ddfa421fd2
Gerrit-Change-Number: 41221
Gerrit-PatchSet: 3
Gerrit-Owner: keith <keith(a)rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith <keith(a)rhizomatica.org>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Fri, 31 Oct 2025 20:15:32 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/pysim/+/41293?usp=email )
Change subject: osmo-smdpp: update commandline help and default port
......................................................................
osmo-smdpp: update commandline help and default port
osmo-smdpp has built-in TLS support for some time now. Let's update
update the commandline help to be more concise.
Since the built-in SSL/TLS support is enabled by default, let's also
update the default port from 8000 to 443.
Change-Id: Ib5a069a8612beb1a9716a7514b498ec70d141178
---
M osmo-smdpp.py
1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/93/41293/1
diff --git a/osmo-smdpp.py b/osmo-smdpp.py
index ed5046d..d1d6fd7 100755
--- a/osmo-smdpp.py
+++ b/osmo-smdpp.py
@@ -861,10 +861,10 @@
def main(argv):
parser = argparse.ArgumentParser()
- parser.add_argument("-H", "--host", help="Host/IP to bind HTTP to", default="localhost")
- parser.add_argument("-p", "--port", help="TCP port to bind HTTP to", default=8000)
+ parser.add_argument("-H", "--host", help="Host/IP to bind HTTP(S) to", default="localhost")
+ parser.add_argument("-p", "--port", help="TCP port to bind HTTP(S) to", default=443)
parser.add_argument("-c", "--certdir", help=f"cert subdir relative to {DATA_DIR}", default="certs")
- parser.add_argument("-s", "--nossl", help="do NOT use ssl", action='store_true', default=False)
+ parser.add_argument("-s", "--nossl", help="disable built in SSL/TLS support", action='store_true', default=False)
parser.add_argument("-v", "--verbose", help="dump more raw info", action='store_true', default=False)
parser.add_argument("-b", "--brainpool", help="Use Brainpool curves instead of NIST",
action='store_true', default=False)
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/41293?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Ib5a069a8612beb1a9716a7514b498ec70d141178
Gerrit-Change-Number: 41293
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>