keith has submitted this change. (
https://gerrit.osmocom.org/c/osmo-msc/+/41221?usp=email )
Change subject: sqlite optimisation: check VLR earlier for dest. subscriber
......................................................................
sqlite optimisation: check VLR earlier for dest. subscriber
sms_from_result() calls a number of sqlite3_column_XXX functions
to build the SMS in memory.
If the destination subscriber is not attached, the sms
will be free'd immediatly anyway in smsq_take_next_sms()
so let's check if the destination subscriber is present
and attached before we call all the sqlite3 routines.
Change-Id: Ibd07b9d200b48108d705ed4c461cc4ddfa421fd2
---
M src/libmsc/db.c
M src/libmsc/sms_queue.c
M tests/sms_queue/sms_queue_test.c
M tests/sms_queue/sms_queue_test.ok
4 files changed, 26 insertions(+), 39 deletions(-)
Approvals:
keith: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/libmsc/db.c b/src/libmsc/db.c
index 04e11c5..43454d6 100644
--- a/src/libmsc/db.c
+++ b/src/libmsc/db.c
@@ -747,6 +747,22 @@
if (!sms)
return NULL;
+ daddr = (const char *)sqlite3_column_text(stmt, COL_DEST_ADDR);
+ if (daddr)
+ OSMO_STRLCPY_ARRAY(sms->dst.addr, daddr);
+
+ if (net != NULL) { /* db_sms_test passes NULL, so we need to be
tolerant */
+ sms->receiver = vlr_subscr_find_by_msisdn(net->vlr,
sms->dst.addr,
+
VSUB_USE_SMS_RECEIVER);
+ if (!sms->receiver || !sms->receiver->lu_complete) {
+ LOGP(DLSMS, LOGL_DEBUG,
+ "Subscriber %s%s is not attached, skipping SMS\n",
+ sms->receiver ? "" : "MSISDN-",
+ sms->receiver ?
vlr_subscr_msisdn_or_name(sms->receiver) : daddr);
+ return sms;
+ }
+ }
+
sms->id = sqlite3_column_int64(stmt, COL_ID);
sms->created = sqlite3_column_int64(stmt, COL_CREATED);
@@ -763,13 +779,6 @@
sms->dst.npi = sqlite3_column_int(stmt, COL_DEST_NPI);
sms->dst.ton = sqlite3_column_int(stmt, COL_DEST_TON);
- daddr = (const char *)sqlite3_column_text(stmt, COL_DEST_ADDR);
- if (daddr)
- OSMO_STRLCPY_ARRAY(sms->dst.addr, daddr);
-
- if (net != NULL) /* db_sms_test passes NULL, so we need to be tolerant
*/
- sms->receiver = vlr_subscr_find_by_msisdn(net->vlr,
sms->dst.addr,
-
VSUB_USE_SMS_RECEIVER);
sms->src.npi = sqlite3_column_int(stmt, COL_SRC_NPI);
sms->src.ton = sqlite3_column_int(stmt, COL_SRC_TON);
diff --git a/src/libmsc/sms_queue.c b/src/libmsc/sms_queue.c
index 7dd14ff..4ef4601 100644
--- a/src/libmsc/sms_queue.c
+++ b/src/libmsc/sms_queue.c
@@ -316,12 +316,7 @@
osmo_strlcpy(last_msisdn, sms->dst.addr, last_msisdn_buflen);
/* Is the subscriber attached? If not, go to next SMS */
- if (!sms->receiver || !sms->receiver->lu_complete) {
- LOGP(DLSMS, LOGL_DEBUG,
- "Subscriber %s%s is not attached, skipping SMS
%llu\n",
- sms->receiver ? "" : "MSISDN-",
- sms->receiver ?
vlr_subscr_msisdn_or_name(sms->receiver)
- : sms->dst.addr, sms->id);
+ if (!sms->id) {
sms_free(sms);
continue;
}
diff --git a/tests/sms_queue/sms_queue_test.c b/tests/sms_queue/sms_queue_test.c
index bca60b1..408c78d 100644
--- a/tests/sms_queue/sms_queue_test.c
+++ b/tests/sms_queue/sms_queue_test.c
@@ -122,13 +122,15 @@
continue;
if (fake_sms_db[i].failed_attempts > max_failed)
continue;
+ if (!fake_sms_db[i].vsub_attached)
+ continue;
sms = sms_alloc();
OSMO_ASSERT(sms);
osmo_strlcpy(sms->dst.addr, fake_sms_db[i].msisdn,
sizeof(sms->dst.addr));
- sms->receiver = fake_sms_db[i].vsub_attached? &arbitrary_vsub :
NULL;
+ sms->id = 1 + i;
osmo_strlcpy(sms->text, fake_sms_db[i].msisdn,
sizeof(sms->text));
if (fake_sms_db[i].vsub_attached)
fake_sms_db[i].nr_of_sms--;
diff --git a/tests/sms_queue/sms_queue_test.ok
b/tests/sms_queue/sms_queue_test.ok
index 146400d..6ccc95f 100644
--- a/tests/sms_queue/sms_queue_test.ok
+++ b/tests/sms_queue/sms_queue_test.ok
@@ -12,21 +12,17 @@
hitting database: looking for MSISDN > '2222', failed_attempts <= 9
#1: sending SMS to 3333 (last_msisdn='3333')
hitting database: looking for MSISDN > '3333', failed_attempts <= 9
- hitting database: looking for MSISDN > '5555', failed_attempts <= 9
hitting database: looking for MSISDN > '', failed_attempts <= 9
#2: sending SMS to 2222 (last_msisdn='2222')
hitting database: looking for MSISDN > '2222', failed_attempts <= 9
#3: sending SMS to 3333 (last_msisdn='3333')
hitting database: looking for MSISDN > '3333', failed_attempts <= 9
- hitting database: looking for MSISDN > '5555', failed_attempts <= 9
hitting database: looking for MSISDN > '', failed_attempts <= 9
-#4: no SMS to send (last_msisdn='5555')
- hitting database: looking for MSISDN > '5555', failed_attempts <= 9
+#4: no SMS to send (last_msisdn='')
hitting database: looking for MSISDN > '', failed_attempts <= 9
-#5: no SMS to send (last_msisdn='5555')
- hitting database: looking for MSISDN > '5555', failed_attempts <= 9
+#5: no SMS to send (last_msisdn='')
hitting database: looking for MSISDN > '', failed_attempts <= 9
-#6: no SMS to send (last_msisdn='5555')
+#6: no SMS to send (last_msisdn='')
- SMS are pending at various nr failed attempts (cutoff at >= 10)
1111 has 1 SMS pending, 0 failed attempts
@@ -60,27 +56,12 @@
5555 (NOT attached) has 1 SMS pending, 0 failed attempts
-->
hitting database: looking for MSISDN > '2345', failed_attempts <= 9
- hitting database: looking for MSISDN > '3333', failed_attempts <= 9
- hitting database: looking for MSISDN > '4444', failed_attempts <= 9
- hitting database: looking for MSISDN > '5555', failed_attempts <= 9
hitting database: looking for MSISDN > '', failed_attempts <= 9
- hitting database: looking for MSISDN > '1111', failed_attempts <= 9
- hitting database: looking for MSISDN > '2222', failed_attempts <= 9
-#0: no SMS to send (last_msisdn='3333')
- hitting database: looking for MSISDN > '3333', failed_attempts <= 9
- hitting database: looking for MSISDN > '4444', failed_attempts <= 9
- hitting database: looking for MSISDN > '5555', failed_attempts <= 9
+#0: no SMS to send (last_msisdn='')
hitting database: looking for MSISDN > '', failed_attempts <= 9
- hitting database: looking for MSISDN > '1111', failed_attempts <= 9
- hitting database: looking for MSISDN > '2222', failed_attempts <= 9
-#1: no SMS to send (last_msisdn='3333')
- hitting database: looking for MSISDN > '3333', failed_attempts <= 9
- hitting database: looking for MSISDN > '4444', failed_attempts <= 9
- hitting database: looking for MSISDN > '5555', failed_attempts <= 9
+#1: no SMS to send (last_msisdn='')
hitting database: looking for MSISDN > '', failed_attempts <= 9
- hitting database: looking for MSISDN > '1111', failed_attempts <= 9
- hitting database: looking for MSISDN > '2222', failed_attempts <= 9
-#2: no SMS to send (last_msisdn='3333')
+#2: no SMS to send (last_msisdn='')
- there are no SMS in the DB
1111 has 0 SMS pending, 0 failed attempts
--
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: merged
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ibd07b9d200b48108d705ed4c461cc4ddfa421fd2
Gerrit-Change-Number: 41221
Gerrit-PatchSet: 4
Gerrit-Owner: keith <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
