[Asterisk-code-review] sorcery/res pjsip: Refactor for realtime performance (asterisk[13])

[George Joseph]

George Joseph has uploaded a new patch set (#2).

Change subject: sorcery/res_pjsip:  Refactor for realtime performance
......................................................................

sorcery/res_pjsip:  Refactor for realtime performance

There were a number of places in the res_pjsip stack that were
getting all endpoints or all aors, and then filtering them locally.

A good example is pjsip_options which, on startup, retrieves all
endpoints, then the aors for those endpoints, then tests the aors
to see if the qualify_frequency is > 0.  One issue was that it never
did anything with the endpoints other than retrieve the aors so we
probably could have skipped a step and just retrieved all aors.
But nevermind.

This worked reasonably well with local config files but with a realtime
backend and thousands of object, this was a nightmare.  The issue really
boiled down to the fact that while realtime supports predicates that
are passed to the database engine, the non-realtime sorcery backends
didn't.

They do now.

The realtime engines have a scheme for doing simple comparisons.
The take an in an ast_variable (or list) for matching and the name
of each variable can contain an operator.  For instance, a name
of "qualify_frequency >" and a value of "0" would create a SQL predicate
that looks like "where qualify_frequency > '0'".  If there's no
operator after the name, the engines add a '=' so a simple
name of "qualify_frequency" and a value of "10" would return exact
matches.

The non-realtime backends decide whether to include an object in a
result set by calling ast_sorcery_changeset_create on every object
in the internal container.  ast_sorcery_changeset_create only does
exact string matches though so a name of "qualify_frequency >" and
a value of "0" returns nothing because the literal "qualify_frequency >"
doesn't match any name in the objset set.

So, the real task was to create a generic string matcher that can
take a left value, operator and a right value and perform the match.
To that end, strings.c has a new ast_strings_match(left, operator, right)
function.  Left anf right are the strings to operate on and the operator
can be a string containing any of the following: = (or NULL or ""), !=, >,
>=, <, <=, like or regex.  If the operator is like or regex, the right
string should be a %-pattern or a regex expression.  If both left and right
can be converted to float, then a numeric comparison is performed, otherwise
a string comparison is performed.

To use this new function, on ast_variables, 2 new functions were added to
config.c.  One that compares 2 ast_variables, and oen that compares 2
ast_variable lists.  The former is useful when you want to compare 2
ast_variables that happen to be in a list but don't want to traverse the
list.  The latter will traverse the right list and return true if all
the variables in the right list match the left list.

Now, the backends' fields_cmp functions call ast_variable_lists_match
instead of ast_sorcery_changeset_create and they can now process the same
syntax as the realtime engines.  The realtime backend just passes the
variable lisst unaltered to the engine.  The only gotcha is that there's
no common realtime engine support for regex so that's been noted in the
api docs for ast_sorcery_retrieve_by_fields.

Only one more change to sorcery was done...  A new config flag
"allow_unqualified_fetch" was added to reg_sorcery_realtime.
"no": ignore fetches if no predicate fields were supplied.
"error": same as no but emit an error. (good for testing)
"yes": allow (the default);
"warn": allow but emit a warning. (good for testing)

Now on to res_pjsip...

pjsip_options was modified to retrieve aors with qualify_frequency > 0
rather than all endpoints then all aors.  Not only was this a big
improvement in realtime retrieval but even for config file there's an
improvement because we're not going through endpoints anymore.

res_pjsip_mwi was modified to retieve only endpoints with something
in the mailboxes field instead of all endpoints then testing mailboxes.

res_pjsip_registrar_expire was completely refactored.  It was
retrieving all contacts then setting up scheduler entries to check
for expiration.  Now, it's a single thread (like keepalive) that
periodically retrieves only contacts whose expiration time is < now
and deletes them.  A new contact_expiration_check_interval was added
to global with a default of 30 seconds.

There are still objects that can't be filtered at the database like identifies,
transports, and registrations.  These are not going to be anywhere near as
numerous as endpoints, aors, auths, contacts however.

Back to allow_unqualified_fetch.  If this is set to yes and you have a very
large number of objects in the database, the pjsip CLI commands will attempt
to retrive ALL of them if not qualified with a LIKE.  Worse, if you type
"pjsip show nedpoint <tab>" guess what's going to happen? :)  Having a cache
helps but all the objects will have to be retrieved at least once to fill
the cache.  Setting allow_unqualified_fetch=no prevents the mass retrieve and
should be used on endpoints, auths, aors, and contacts.  It should NOT be
used for identifies, registrations and transports and these MUST be
retrieved in bulk.

Example sorcery.conf:

[res_pjsip]
endpoint=config,pjsip.conf,criteria=type=endpoint
endpoint=realtime,ps_endpoints,allow_unqualified_fetch=error

ASTERISK-25826 #close
Reported-by: Ross Beer

Change-Id: Id2691e447db90892890036e663aaf907b2dc1c67
---
M configs/samples/pjsip.conf.sample
A contrib/ast-db-manage/config/versions/5813202e92be_add_contact_expiration_check_interval_.py
M include/asterisk/config.h
M include/asterisk/res_pjsip.h
M include/asterisk/sorcery.h
M include/asterisk/strings.h
M main/config.c
M main/strings.c
M res/res_pjsip.c
M res/res_pjsip/config_global.c
M res/res_pjsip/pjsip_distributor.c
M res/res_pjsip/pjsip_options.c
M res/res_pjsip_mwi.c
M res/res_pjsip_registrar_expire.c
M res/res_sorcery_config.c
M res/res_sorcery_memory.c
M res/res_sorcery_memory_cache.c
M res/res_sorcery_realtime.c
M tests/test_strings.c
19 files changed, 610 insertions(+), 292 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/70/2370/2
-- 
To view, visit https://gerrit.asterisk.org/2370
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id2691e447db90892890036e663aaf907b2dc1c67
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>