[asterisk-dev] [Code Review] Patch pbx_dundi.c to periodically clean it's database cache

Matthew Nicholson mnicholson at digium.com
Mon Dec 1 11:07:31 CST 2008



> On 2008-11-26 09:17:16, Russell Bryant wrote:
> > /branches/1.6.0/pbx/pbx_dundi.c, line 2169
> > <http://reviewboard.digium.com/r/63/diff/2/?file=1661#file1661line2169>
> >
> >     Is the missing leading / intentional?  If it doesn't matter, I'd add it for clarity.

Yes.  The missing / is intentional.  I think you have to leave it off for ast_db_gettree() to work properly.


> On 2008-11-26 09:17:16, Russell Bryant wrote:
> > /branches/1.6.0/pbx/pbx_dundi.c, lines 4827-4830
> > <http://reviewboard.digium.com/r/63/diff/2/?file=1661#file1661line4827>
> >
> >     The way that this code tries to stop the thread from running is problematic.  This is only going to work if the thread is in sleep() at this point.  If it is in the middle of processing, the module unload is going to block for a minute.
> >     
> >     Instead, I would use a pthread lock and condition to make sure that you don't get stuck in a sleep.
> >     
> >     In the thread loop:
> >     
> >       lock()
> >       check to see if i should quit;
> >       sleep on the condition for a minute;
> >       unlock();
> >     
> >     In unload:
> >     
> >       lock()
> >       signal in case the thread is sleeping.
> >       unlock()

I will look into this.


- Matthew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/63/#review162
-----------------------------------------------------------


On 2008-11-24 10:00:37, Matthew Nicholson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/63/
> -----------------------------------------------------------
> 
> (Updated 2008-11-24 10:00:37)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This is a modified version of the patch from bug 13819 (http://bugs.digium.com/view.php?id=13819).  It spawns a thread to remove expired entries from the dundi cache (in astdb) every 60 seconds.  This patch is necessary because asterisk normally cleans these entries when they are looked up, but entries that are not retrieved often (or retrieved only once ever) will stay in the database.
> 
> 
> This addresses bug 13819.
>     http://bugs.digium.com/view.php?id=13819
> 
> 
> Diffs
> -----
> 
>   /branches/1.6.0/pbx/pbx_dundi.c 158369 
> 
> Diff: http://reviewboard.digium.com/r/63/diff
> 
> 
> Testing
> -------
> 
> I tested this by creating some expired entries in the database using 'database put dundi/cache test 0|0' and observed asterisk clean them up.
> 
> 
> Thanks,
> 
> Matthew
> 
>




More information about the asterisk-dev mailing list