[Asterisk-code-review] res_pjsip_registrar: Remove unavailable contacts if exceeds max_contacts (asterisk[16])

[Kevin Harwell]

Attention is currently required from: Sean Bright, Joshua Colp, Joe.
Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/16160 )

Change subject: res_pjsip_registrar: Remove unavailable contacts if exceeds max_contacts
......................................................................


Patch Set 7:

(3 comments)

File res/res_pjsip_registrar.c:

https://gerrit.asterisk.org/c/asterisk/+/16160/comment/7f66f34e_9f43f61b 
PS7, Line 593: 		if (!remove_existing) {
             : 			registrar_contact_delete(CONTACT_DELETE_UNAVAILABLE, NULL, contact, contact->aor);
             : 		} else {
             : 			registrar_contact_delete(CONTACT_DELETE_EXISTING, NULL, contact, contact->aor);
             : 		}
I'm not sure if this is correct. If the given contacts is a list of unavailable contacts, and remove_existing is true then unavailable contacts are deleted with a reason of "remove existing" but are being removed due to unavailable.

The reverse happens if existing expired ones are passed in but "existing" is false.


https://gerrit.asterisk.org/c/asterisk/+/16160/comment/cf22f451_6a49969f 
PS7, Line 718: ast_sip_location_retrieve_aor_contacts_filtered(aor, AST_SIP_CONTACT_FILTER_UNREACHABLE);
> This function returns all un-expired, unreachable contacts, both dynamic and permanent. […]
Actually looking at the registrar_add_non_permanent function it appears to only add non-expired contacts, so you'd need to retrieve another separate list of dynamic unavail ones


https://gerrit.asterisk.org/c/asterisk/+/16160/comment/3908dff9_bbb82ccb 
PS7, Line 927: contact_count = ao2_container_count(existing_contacts) + updated + added;
             : 		if (contact_count > aor->max_contacts) {
             : 			/* Remove excess existing contacts that are unavailable or expire soonest */
             : 			remove_excess_contacts(existing_contacts, contacts, contact_count - aor->max_contacts,
             : 				aor->remove_existing);
             : 		}
If unavailable contacts have been removed should the existing contacts now be updated and then checked to see if the contact_count is below max contacts? Or maybe keep track of how many were removed during unavail removal and use in the calculation here?

Or should it remain, and potentially unavailable and expired contacts are removed even if unavailable lowered the number to be below max?



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/16160
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ia2711b08f2b4d1177411b1be23e970d7fdff5784
Gerrit-Change-Number: 16160
Gerrit-PatchSet: 7
Gerrit-Owner: Joe <ynadiv at corpit.xyz>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean at seanbright.com>
Gerrit-Attention: Sean Bright <sean at seanbright.com>
Gerrit-Attention: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Joe <ynadiv at corpit.xyz>
Gerrit-Comment-Date: Wed, 11 Aug 2021 16:49:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210811/6fc2a531/attachment.html>