From 07ebcf8f2410e8d0329d6645804f6d9e2c78f41f Mon Sep 17 00:00:00 2001 From: Victor Seva Date: Wed, 29 Jun 2022 09:09:29 +0200 Subject: [PATCH] TT#178351 presence: be more resilient doing clean up of presentity previously if an error was found we were bailing out and the value was kept so at next round the value will be there and no more values where removed Change-Id: I33bb45f5593dad43f3b5b8b962c77fe99c6f0e38 --- debian/patches/series | 3 +- ...nce-be-more-resilient-doing-clean-up.patch | 139 ++++++++++++++++++ 2 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 debian/patches/sipwise/presence-be-more-resilient-doing-clean-up.patch diff --git a/debian/patches/series b/debian/patches/series index 85b0518f1..507e5f8f1 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -56,8 +56,9 @@ sipwise/db_redis_sscan_fix_empty_key.patch sipwise/kamctl-TMPDIR-config.patch sipwise/lcr-stopper_mode-parameter.patch sipwise/dialog-support-profile_get_size-for-all-profiles.patch -### active development sipwise/pv_headers-rework-pvh_remove_header_param.patch +### active development +sipwise/presence-be-more-resilient-doing-clean-up.patch # ### Don't just put stuff in any order ### use gbp pq import/export tooling to help maintain patches diff --git a/debian/patches/sipwise/presence-be-more-resilient-doing-clean-up.patch b/debian/patches/sipwise/presence-be-more-resilient-doing-clean-up.patch new file mode 100644 index 000000000..e520f6160 --- /dev/null +++ b/debian/patches/sipwise/presence-be-more-resilient-doing-clean-up.patch @@ -0,0 +1,139 @@ +From: Victor Seva +Date: Wed, 29 Jun 2022 09:03:14 +0200 +Subject: presence: be more resilient doing clean up of presentity values + +previously if an error was found we were bailing out and the value +was kept so at next round the value will be there and no more values +where removed +--- + src/modules/presence/publish.c | 59 ++++++++++++++++++++++++++---------------- + 1 file changed, 37 insertions(+), 22 deletions(-) + +diff --git a/src/modules/presence/publish.c b/src/modules/presence/publish.c +index 8889a2e..9bd334f 100644 +--- a/src/modules/presence/publish.c ++++ b/src/modules/presence/publish.c +@@ -69,6 +69,7 @@ void ps_presentity_db_timer_clean(unsigned int ticks, void *param) + int n_db_cols = 0, n_result_cols = 0; + int event_col, etag_col, user_col, domain_col; + int i = 0, num_watchers = 0; ++ pres_ev_t fake; + presentity_t pres; + str uri = {0, 0}, event, *rules_doc = NULL; + static str query_str; +@@ -121,6 +122,7 @@ void ps_presentity_db_timer_clean(unsigned int ticks, void *param) + rows = RES_ROWS(result); + + for(i = 0; i < RES_ROW_N(result); i++) { ++ num_watchers = 0; + values = ROW_VALUES(&rows[i]); + memset(&pres, 0, sizeof(presentity_t)); + +@@ -134,28 +136,33 @@ void ps_presentity_db_timer_clean(unsigned int ticks, void *param) + event.len = strlen(event.s); + pres.event = contains_event(&event, NULL); + if(pres.event == NULL || pres.event->evp == NULL) { +- LM_ERR("event not found\n"); +- goto error; ++ LM_ERR("event[%.*s] not found\n", STR_FMT(&event)); ++ memset(&fake, 0, sizeof(pres_ev_t)); ++ fake.name = event; ++ pres.event = &fake; ++ goto simple_error; + } + + if(uandd_to_uri(pres.user, pres.domain, &uri) < 0) { +- LM_ERR("constructing uri\n"); +- goto error; ++ LM_ERR("constructing uri from [user]=%.*s [domain]=%.*s\n", ++ STR_FMT(&pres.user), STR_FMT(&pres.domain)); ++ goto simple_error; + } + + /* delete from hash table */ + if(publ_cache_mode==PS_PCACHE_HYBRID + && delete_phtable(&uri, pres.event->evp->type) < 0) { +- LM_ERR("deleting from presentity hash table\n"); +- goto error; ++ LM_ERR("deleting uri[%.*s] event[%.*s] from presentity hash table\n", ++ STR_FMT(&uri), STR_FMT(&event)); ++ goto simple_error; + } + +- LM_DBG("found expired publish for [user]=%.*s [domanin]=%.*s\n", ++ LM_DBG("found expired publish for [user]=%.*s [domain]=%.*s\n", + pres.user.len, pres.user.s, pres.domain.len, pres.domain.s); + + if(pres_force_delete == 1) { + if(delete_presentity(&pres, NULL) < 0) { +- LM_ERR("Deleting presentity\n"); ++ LM_ERR("Deleting presentity uri[%.*s]\n", STR_FMT(&uri)); + goto error; + } + } else if(pres_notifier_processes > 0) { +@@ -171,7 +178,7 @@ void ps_presentity_db_timer_clean(unsigned int ticks, void *param) + if(pa_dbf.abort_transaction(pa_db) < 0) + LM_ERR("in abort_transaction\n"); + } +- goto error; ++ goto next; + } + + if(num_watchers > 0) { +@@ -181,12 +188,12 @@ void ps_presentity_db_timer_clean(unsigned int ticks, void *param) + if(pa_dbf.abort_transaction(pa_db) < 0) + LM_ERR("in abort_transaction\n"); + } +- goto error; ++ goto next; + } + } else { + if(delete_presentity(&pres, NULL) < 0) { +- LM_ERR("Deleting presentity\n"); +- goto error; ++ LM_ERR("Deleting presentity uri[%.*s]\n", STR_FMT(&uri)); ++ goto next; + } + } + if(pa_dbf.end_transaction) { +@@ -201,22 +208,30 @@ void ps_presentity_db_timer_clean(unsigned int ticks, void *param) + &pres.user, &pres.domain, &rules_doc) + < 0) { + LM_ERR("getting rules doc\n"); +- goto error; ++ goto simple_error; + } + if(publ_notify(&pres, uri, NULL, &pres.etag, rules_doc) < 0) { + LM_ERR("sending Notify request\n"); +- goto error; +- } +- if(rules_doc) { +- if(rules_doc->s) +- pkg_free(rules_doc->s); +- pkg_free(rules_doc); +- rules_doc = NULL; ++ goto simple_error; + } + } + +- pkg_free(uri.s); +- uri.s = NULL; ++simple_error: ++ if(num_watchers == 0 && delete_presentity(&pres, NULL) < 0) { ++ LM_ERR("Deleting presentity\n"); ++ } ++next: ++ if(uri.s) { ++ pkg_free(uri.s); ++ uri.s = NULL; ++ } ++ if(rules_doc) { ++ if(rules_doc->s) { ++ pkg_free(rules_doc->s); ++ } ++ pkg_free(rules_doc); ++ rules_doc = NULL; ++ } + } + } while(db_fetch_next(&pa_dbf, pres_fetch_rows, pa_db, &result) == 1 + && RES_ROW_N(result) > 0);