[Asterisk-code-review] res pjsip: Deny requests when threadpool queue is backed up. (asterisk[13])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: res_pjsip: Deny requests when threadpool queue is backed up.
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/1607/1/res/res_pjsip/pjsip_distributor.c
File res/res_pjsip/pjsip_distributor.c:

Line 249: #define SIP_MAX_QUEUE 500l
> I'm not sure I understand this finding. The max queue size isn't actually a
Sorry, I should have explained it better.

In taskprocessor.c, we've defined the high water mark as HIGH_WATER_LEVEL (which equates to 100). When that boundary is exceeded, we'll emit an ERROR message indicating that the queue is overloaded. Since that is #defined in the implementation of the taskprocessor routines, that is an implementation detail of taskprocessors currently that is inaccessible outside of that translation unit.

Here, we've defined another max size after which we will reject SIP requests - which equates to 500. Personally, I'd prefer to see the two linked somehow. If we find that the HIGH_WATER_LEVEL is too low or too high, that should probably have an affect on when res_pjsip determines that it should start rejecting SIP requests. Since those two #defines are currently mutually exclusive, changing one won't effect the other, when it probably should.

I would:
(1) Define HIGH_WATER_LEVEL in taskprocessor.h
(2) Make SIP_MAX_QUEUE either equal HIGH_WATER_LEVEL or a multiple of it (probably a multiple). If we think the two should be equal, than HIGH_WATER_LEVEL should probably be 500.


-- 
To view, visit https://gerrit.asterisk.org/1607
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4e736d48a2ba79fd1f8056c0dcd330e38e6a3816
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes