keith has uploaded this change for review.
SMS optimise: Do VLR check for destination subscriber earlier
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, 31 insertions(+), 42 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/21/41221/1
diff --git a/src/libmsc/db.c b/src/libmsc/db.c
index 04e11c5..0c5c543 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..86a0e39 100644
--- a/src/libmsc/sms_queue.c
+++ b/src/libmsc/sms_queue.c
@@ -301,11 +301,13 @@
while (wrapped < 2 && (--sanity)) {
/* If we wrapped around and passed the first msisdn, we're
* through the entire SMS DB; end it. */
- if (wrapped && strcmp(last_msisdn, started_with_msisdn) >= 0)
+ if (wrapped && strcmp(last_msisdn, started_with_msisdn) >= 0) {
+ DEBUGP(DLSMS, "SMS queue: Reached end of DB.\n");
break;
-
+ }
sms = db_sms_get_next_unsent_rr_msisdn(net, last_msisdn, 9);
if (!sms) {
+ DEBUGP(DLSMS, "SMS queue: Got no next unsent SMS\n");
last_msisdn[0] = '\0';
wrapped++;
continue;
@@ -316,12 +318,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;
}
@@ -329,7 +326,7 @@
return sms;
}
- DEBUGP(DLSMS, "SMS queue: no SMS to be sent\n");
+ DEBUGP(DLSMS, "SMS queue: no SMS to be sent, tried %d times.\n", sanity+100);
return NULL;
}
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 change 41221. To unsubscribe, or for help writing mail filters, visit settings.