[asterisk-dev] [Code Review] Make reloading cdr_pgsql.so not screw up catastrophically.

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1711/#review5403
-----------------------------------------------------------



/branches/1.8/cdr/cdr_pgsql.c
<https://reviewboard.asterisk.org/r/1711/#comment9971>

    I was thinking about this, and I think you could simplify things greatly by locking pgsql_lock whether this is a reload or not. This way, you don't have the odd conditional unlock function and you can just always unlock at return points. Locking the pgsql_lock during the initial load doesn't really accomplish much, but it doesn't hurt anything either, so I say just go ahead and simplify things here.


- Mark


On Feb. 3, 2012, 11:11 a.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1711/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2012, 11:11 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Without this patch, attempting cli command:
> module reload cdr_pgsql.so
> 
> will cause the columns list to get doubled up (because they aren't cleared) which will make insertions fail since the insert statement will have all of its elements repeated.
> 
> This patch does two things.  Well, three.  First it unregisters cdr_pgsql from cdr.c so that cdr logs won't try to use this cdr storage method while the reload process is happening (re-registration occurs at the end of the reload).  Second, it clears the columns list during reload so that it doesn't end up doubling up the entires.  As an irrelevant third, it updates the copyright status in the documentation to 2012 from 2006 or so.
> 
> 
> This addresses bug ASTERISK-19216.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19216
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/cdr/cdr_pgsql.c 353844 
> 
> Diff: https://reviewboard.asterisk.org/r/1711/diff
> 
> 
> Testing
> -------
> 
> Tested with and without patch. With patch, the CDRs get logged correctly after reloading without the errors mentioned in the bug report and also the reload doesn't spit out a "this cdr backend has already been registered in cdr.c" (paraphrasing) type error.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120203/6ae2357f/attachment.htm>