#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [x] Commit message has the format required by CONTRIBUTING guide - [x] Commits are split per component (core, individual modules, libs, utils, ...) - [x] Each component has a single commit (if not, squash them into one commit) - [x] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [x] Small bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [x] PR should be backported to stable branches - [ ] Tested changes locally - [ ] Related to issue #2909
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/2910
-- Commit Summary --
* <a href="https://github.com/kamailio/kamailio/pull/2910/commits/0b424ca7b7a8ac7f2e3e94a238344fe8b4fa8296">permissions: implement lock for trusted hash</a>
-- File Changes --
M src/modules/permissions/permissions.c (6) M src/modules/permissions/rpc.c (6) M src/modules/permissions/trusted.c (16) M src/modules/permissions/trusted.h (4)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/2910.patch https://github.com/kamailio/kamailio/pull/2910.diff
I am not using permissions with trusted table, but it looks like now it's going to be no parallel use of the in-memory table. It's safe against reloads done quickly one after the other, but all kamailio processes that do permissions check wait one after the other always.
If one ensures the reloads are not triggered quickly one after the other, then the patch is slowing down, specially if the number of records is high. An option would be to add a mod param to enable this lock or not at startup.
Anyhow, as I said, I am not using this feature, so I can't evaluate the impact. Maybe @juha-h can comment if he uses this feature, or some other devs.
Yes, you're right. I didn't realize I was destroying parallel use of the hash. I was focusing on be sure the memory was not going to go away in the middle of a search. I need to find a better solution.
Iirc, with other rpc commands related to db reload it was introduced a "safety delay" for them, so they cannot be done very often.
Yes, I implemented the delta parameter [0] but that doesn't help if you're in the middle of a search
https://github.com/kamailio/kamailio/commit/01fa7503433f4e23dea156e1034a87bb...
Some modules keep also the old data till next reload (dispatcher ?!?), I thought is the case also here.
Then the solution could be to have a list of old hash tables that are destroyed on timer, after some configurable delay. Practically, instead of destroying immediately, add it to the list with a timestamp, and destroy after N secs with a timer callback.
Yes, that would be a nice approach. I'm going to implement it like that.
Thank you!
@linuxmaniac pushed 1 commit.
7700a8d8239f4078366fb8cd6aa2f3555ec1ef6c permissions: don't remove old data at the end of the reload process
@linuxmaniac pushed 1 commit.
7ce815c2612d6277b55aabf6519c70a4ecec6502 permissions: trusted_cleanup_interval
I guess there is no possibility to perform another reload as long as the old hash table is not freed on timer. If yes, then fine for me, the new modparm has to be documented, if not done.
I guess there is no possibility to perform another reload as long as the old hash table is not freed on timer. If yes, then fine for me
[here](https://github.com/kamailio/kamailio/pull/2910/commits/7ce815c2612d6277b55aa...) I check that no reload was done before last tick.
if a reload is called before the timer interval, nothing changed. [reload_trusted_table](https://github.com/kamailio/kamailio/blob/7ce815c2612d6277b55aabf6519c70a4ec...) It was already removing the old previous values before loading new values there before the switch.
the new modparm has to be documented, if not done.
To be added if we agree in the implementation
Docs can be added and then merged.
Merged #2910 into master.
-
Daniel-Constantin Mierla -
Victor Seva