* defaults to 0 * adds X seconds to expires before being stored in htable or database * causes subscribes to not be removed prematurely You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/654
-- Commit Summary --
* presence: add new parameter `subscribe_expires_offset`
-- File Changes --
M modules/presence/doc/presence_admin.xml (20) M modules/presence/presence.c (67) M modules/presence/presence.h (1) M modules/presence/subscribe.c (2)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/654.patch https://github.com/kamailio/kamailio/pull/654.diff
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654
@miconda This resolved an on-going issue we noticed with BLF line keys losing presence (on polycoms we saw the line keys turning gray). We noticed it was happening because the Polycom was not re-subscribing before the expiration and therefore, kamailio was deleting the subscription from the database (as expected). This patch adds X seconds to the expiration to give the UA more time to re-subscribe before deleting from the database. Please let me know if you have any questions regarding this new parameter. Thanks, Emmanuel
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-223820610
@eschmidbauer doesn't `expires_offset ` already addresses this ?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-223820687
@lazedo It does not. Reading the doc, `expires_offset` seems to only affect PUBLISH requests. Testing and examining the code, I found that `expires_offset` had not affect on SUBSCRIBE requests.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-223820783
@eschmidbauer please check again. in subscribe.c the timers for subscription updates already take `expires_offset`. maybe there's a code path that is not taking this parameter ?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-223821591
@lazedo According to the doc on `expires_offset`: The value in seconds that should be subtracted from the expires value when sending a 200OK for a publish. It is used for forcing the client to send an update before the old publish expires.
This is not what we are attempting to resolve with this PR. We only want to add X seconds to the expires before inserting into db/htable. This gives extra time for the UA to re-SUBSCRIBE before kamailio removes from db/htable.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-223821817
@eschmidbauer please read the code related to presence timers, they make the select with the expires_offset. the change that you're proposing, it seems to me, that will have the side effect on NOTIFY messages of sending the wrong remaining period in the subscription, if the UA takes the value from the NOTIFY then you will end up with the same problem again.
if you think that you found that expires_offset has no affect on SUBSCRIBE requests, please show us where. i see it being used in `update_db_subs_timer_notifier` , `update_db_subs_timer_dbonly`, `update_db_subs_timer_dbnone`, `update_db_subs_timer`. current_time is deducted the expires_offsite and gets subscription LT that value. check here [here](https://github.com/kamailio/kamailio/blob/b0a0b3dd12b1c218fffae75b9da7d454d1...)
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-223843584
test comment for sr-dev mail, please ignore
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-224027922
@lazedo I removed expires_offset so now i have: ``` #modparam("presence", "expires_offset", 60) modparam("presence", "subscribe_expires_offset", 60) ```
I will test the following on our staging ``` modparam("presence", "expires_offset", 120) #modparam("presence", "subscribe_expires_offset", 60) ```
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-224033650
@lazedo update: our Polycom on staging is losing presence with the following configuration ``` modparam("presence", "expires_offset", 120) //modparam("presence", "subscribe_expires_offset", 60) ```
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-224097656
@eschmidbauer any logs to sustain that ?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-224107209
@eschmidbauer btw, which `db_mode` are you using in your config ?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-224112568
@lazedo ```modparam("presence", "subs_db_mode", 3)``` I'll try to get logs but it's difficult as the problem is sporadic
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-224302172
Still no time to look deeper at the issues, but at least the new patch c7378df is better -- no longer very intrusive, with minimal impact on performance and the new parameter reflects better the fact that it adds to the expires. If it proves to solve issues when dealing with some phones, I am ok to merge provided that default behaviour is to do no change to the expires. But I would like to see more into its description in the docs, because it seems to affect only subscriptions. Moreover, it would be good to say if it conflicts or not with the expires_offset.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-224607166
@miconda @lazedo I'm canceling this PR for now. We are testing some settings on Polycom and we want to further analyze before we are sure this code is needed, thanks. Emmanuel
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#issuecomment-224728927
Closed #654.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/654#event-686451361
-
Daniel-Constantin Mierla -
Emmanuel Schmidbauer
-
lazedo
-
Victor Seva