presence module triggering postgres constraint when marking records for deletion

[eschmidbauer]

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

[eschmidbauer]

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

[eschmidbauer]

@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:
16215a8

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.

[juha-h]

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

[miconda]

@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.

[miconda]

@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.

[miconda]

PR #601 was merged.