Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kamctl:Index "inserted_time"+"status" in watchers #3182

Merged
merged 2 commits into from Jul 21, 2022
Merged

kamctl:Index "inserted_time"+"status" in watchers #3182

merged 2 commits into from Jul 21, 2022

Conversation

Ozzyboshi
Copy link

Pre-Submission Checklist

  • Commit message has the format required by CONTRIBUTING guide
  • Commits are split per component (core, individual modules, libs, utils, ...)
  • Each component has a single commit (if not, squash them into one commit)
  • 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

  • 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:

  • PR should be backported to stable branches
  • Tested changes locally
  • Related to issue #XXXX (replace XXXX with an open issue number)

Description

Function ps_watchers_db_timer_clean() inside src/modules/presence/subscribe.c is called repeatedly on my setup.
Each time is called it performs a delete inside table watchers comparing columns inserted_time and status which are not indexed:

MariaDB [kamailio]> explain delete from `watchers` where `inserted_time`<1656252889 AND `status`=2;
+------+-------------+----------+------+---------------+------+---------+------+------+-------------+
| id   | select_type | table    | type | possible_keys | key  | key_len | ref  | rows | Extra       |
+------+-------------+----------+------+---------------+------+---------+------+------+-------------+
|    1 | SIMPLE      | watchers | ALL  | NULL          | NULL | NULL    | NULL | 1    | Using where |
+------+-------------+----------+------+---------------+------+---------+------+------+-------------+

Can we consider adding an index improving performance a bit?

MariaDB [kamailio]> CREATE INDEX inserted_time_status_idx ON watchers (`inserted_time`, `status`);
Query OK, 0 rows affected (0.015 sec)
Records: 0  Duplicates: 0  Warnings: 0

MariaDB [kamailio]> explain delete from `watchers` where `inserted_time`<1656252889 AND `status`=2;
+------+-------------+----------+-------+--------------------------+--------------------------+---------+------+------+-------------+
| id   | select_type | table    | type  | possible_keys            | key                      | key_len | ref  | rows | Extra       |
+------+-------------+----------+-------+--------------------------+--------------------------+---------+------+------+-------------+
|    1 | SIMPLE      | watchers | range | inserted_time_status_idx | inserted_time_status_idx | 4       | NULL | 1    | Using where |
+------+-------------+----------+-------+--------------------------+--------------------------+---------+------+------+-------------+
1 row in set (0.001 sec)

@miconda
Copy link
Member

miconda commented Jul 12, 2022

Thanks for the PR!

But the index definition has to be done in the xml files that specify the schema src/lib/srdb1/schema/pr_watchers.xml -- repo link:

The the sql/db creation scripts can be generated for all backends (mysql, postgres, ...) with:

make dbschema

You would need xml tools installed, you can just change the xml file and I can then regenerated the sql/db scripts.

You can force push to this PR or close and open another one.

Alessio Garzi added 2 commits July 14, 2022 09:53
- New index for watchers table for columns "inserted_time" and "status".
  This is a little performance boost since the function
  ps_watchers_db_timer_clean() cleans pending subscriptions
  using this columns inside the "where" clause.
  New index has been added to both Postgres and Mysql/MariaDB backends.
- regenerated db schema files for table watchers
  after addition of index time_status_idx
@Ozzyboshi
Copy link
Author

I changed the xml file and regenerated the sql files in 2 separate commits please let me know if it's ok

@henningw henningw merged commit 45c3861 into kamailio:master Jul 21, 2022
@henningw
Copy link
Contributor

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants