[kamailio/kamailio] presence module triggering postgres constraint when marking records for deletion (#600)
We are using the `presence` module and have noticed many errors in the log as follows: ```WARNING: db_postgres [km_dbase.c:240]: db_postgres_submit_query(): postgres result check failed with code 7 (PGRES_FATAL_ERROR) WARNING: db_postgres [km_dbase.c:244]: db_postgres_submit_query(): postgres query command failed, connection status 0, error [ERROR: duplicate key value violates unique constraint "presentity_presentity_idx"#012DETAIL: Key (username, domain, event, etag)=(user+test2, phonedev.test2.io, dialog, *#-OFFLINE-#*) already exists.#012]```
I looked over the `presentity.c` code and it appears that the module is marking the records for deletion once they expire by setting the `etag` to `*#-OFFLINE-#*`
This is triggering the postgres constraint and throwing the error. I am confused why the module is using the `etag` column to mark for deletion since it is part of a constraint. Why not add a new column such as `delete_record` and set it to true?
If there is no good reason for this, I would like to propose a patch to add said column. Please let me know if I am misunderstanding anything here, 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/issues/600
After further investigation of the presence module, I found that the function inside publish.c will delete (instead of mark for deletion) records if there are no watchers. For our purposes, we will not be using watchers so I'd like to propose a presence module setting that will force deletion of expired records in presentity table rather than mark them for deletion. 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/issues/600#issuecomment-217252895
@miconda I'd like to introduce a new parameter to the presence module, `force_delete` This parameter (disabled by default) will delete (not mark for deletion) expired presentity records without sending a NOTIFY. We control presence using the NSQ module and found the presence module was conflicting with our use case. The presence module is marking expired presentity records for deletion and in addition sending terminated NOTIFYs. It's causing database constraint issues and also early termination of dialog-info. Please see my initial commit and let me know chances of getting this patch in: https://github.com/kamailio/kamailio/commit/16215a83b123ba8b4f86d34368c36dd5...
I apologize for the whitespace clean up, my text editor automatically performed the cleanup. Thanks Emmanuel
We require both these features explicitly disabled which is why we are introducing this patch.
--- 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/issues/600#issuecomment-217273576
Emmanuel Schmidbauer writes:
I'd like to introduce a new parameter to the presence module, `force_delete` This parameter (disabled by default) will delete (not mark for deletion) expired presentity records without sending a NOTIFY. We control presence using the NSQ module and found the presence module was conflicting with our use case.
presence module implements rfc 3856. i don't like the idea of extending it to do something else.
-- juha
--- 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/issues/600#issuecomment-217356690
@juha-h: this is not changing anything in terms of the SIP specs.
The behaviour is there, but now it is only a matter of the notifier_processes parameter. Given that this parameter is also useful for other tasks, I think it is fine to have a condition on another parameter for more fine tunings of the behaviour. The change is minimal and the default value preserves the current behaviour, so I think it is useful to have.
@eschmidbauer: next time make a commit for code reformatting and white spacing fixes. I find them really useful and I do them, but if you combine them with other changes, it is not easy to spot the real changes.
--- 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/issues/600#issuecomment-217372102
@eschmidbauer: make a pull request, to be easier to review over all. You can make pull requests from branches of kamailio, no need to clone repo.
--- 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/issues/600#issuecomment-217423673
PR #601 was merged.
--- 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/issues/600#issuecomment-217612968
Closed #600.
--- 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/issues/600#event-653369339
-
Daniel-Constantin Mierla -
Emmanuel Schmidbauer
-
juha-h