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

[jrose]

> On Feb. 3, 2012, 2:26 p.m., rmudgett wrote:
> > /branches/1.8/cdr/cdr_pgsql.c, lines 663-668
> > <https://reviewboard.asterisk.org/r/1711/diff/3/?file=23883#file23883line663>
> >
> >     If you move the ast_cdr_register to load_module, you can simplify this code since only loading the module needs to register.

I think the simplifications here are actually a lot more costly in complications below. ast_cdr_register has its own failure conditions and I'd basically have to come up with a new return value to indicate that I actually would have performed ast_cdr_register from this function which would just have to be evaluated in the load_module function anyway.


- jrose


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


On Feb. 3, 2012, 1:05 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1711/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2012, 1:05 p.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 353998 
> 
> 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/7af132e3/attachment.htm>