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

[Russell Bryant]

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



/branches/1.6.0/pbx/pbx_dundi.c
<http://reviewboard.digium.com/r/63/#comment280>

    Is the missing leading / intentional?  If it doesn't matter, I'd add it for clarity.



/branches/1.6.0/pbx/pbx_dundi.c
<http://reviewboard.digium.com/r/63/#comment281>

    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()


- Russell


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
> 
>