keith has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/41221?usp=email ) Change subject: SMS optimise: Do VLR check for destination subscriber earlier ...................................................................... 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 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: newchange Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ibd07b9d200b48108d705ed4c461cc4ddfa421fd2 Gerrit-Change-Number: 41221 Gerrit-PatchSet: 1 Gerrit-Owner: keith &lt;keith(a)rhizomatica.org&gt;