[Asterisk-code-review] Add support for delayed shutdown of Asterisk when checking f... (testsuite[master])

[Corey Farrell]

Corey Farrell has posted comments on this change. ( https://gerrit.asterisk.org/6845 )

Change subject: Add support for delayed shutdown of Asterisk when checking for leaks.
......................................................................


Patch Set 3:

> I'm not a real fan of having to add a parameter to every test's
 > yaml.  How about making this a command line option that
 > automatically adds
 > 'n' amount to very reactor timeout?
 > 
 > --reactor-timeout-adjustment=5 to add 5 seconds, or if you have a
 > really fast machine,  --reactor-timeout-adjustment=-5 to reduce the
 > timeout by 5 seconds.

I think you're misunderstanding the purpose of this patch.  Reactor timeout is a side-effect, not the main point.  Reactor timeout is only touched to deal with the case where a test needs >30 second shutdown delay.  So if we have memcheck-delay-stop: 90 to deal with a minse timer we increase the reactor-timeout to 93 seconds minimum to avoid showing reactor timeout messages while the test is stopping.

This patch is not intended to deal with any slowness caused by valgrind or REF_DEBUG (code to do that globally is already in place).  The point is to wait long enough for any relevant timers to fire and cleanup resources.  SIP timer_b takes just as long to fire with or without memory checking, the only difference is that we care enough to wait for it when we are checking memory.

In my own testing this memcheck-delay-stop will only be needed on about half the PJSIP tests and is unneeded by nearly all non-PJSIP tests.  Many of the PJSIP tests will need 7 seconds (assuming timer_b is configured to minimum).  Some tests will need 60 seconds (ast_sip_session_defer_termination), others might need 90 seconds (minse configured to minimum).  I had considered setting the default memcheck-delay-stop to 7 seconds but this would have drawbacks.  A 7 second delay for every test would increase the run-time of the full testsuite by nearly 2 hours (1000 tests).  Maybe 200 of the tests need any delay.  Most cases we're dealing with timer_b which means that we need to reconfigured timer_t1 + timer_b or use memcheck-delay-stop: 32, so for a 7 second delay to ever be effective we'd need to touch the pjsip.conf files anyways.


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

Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I67ec89e4a116526ad34796a233e15fec794cf3e1
Gerrit-Change-Number: 6845
Gerrit-PatchSet: 3
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Mon, 30 Oct 2017 15:10:07 +0000
Gerrit-HasComments: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171030/2c504f54/attachment.html>