[asterisk-dev] [Code Review] res_config_pgsql needs db reconnection support

[kobaz]

> On 2010-11-24 03:52:05, Olle E Johansson wrote:
> > trunk/res/res_config_pgsql.c, line 149
> > <https://reviewboard.asterisk.org/r/944/diff/2/?file=12629#file12629line149>
> >
> >     Should we use PostgreSQL or just "postgres" - we have a mix in the messages here. (Check line 166)
> >     
> >     Also - is it "not defined" or "not active" ? Well it's just a debug message.. But anyway.
> 
> kobaz wrote:
>     Sounds good... I'll make everything PostgreSQL.

Not active might be a good descriptive term.  The current message states the exact nature of the connection handle... which is that it's not a handle at all since it's currently NULL.  I think not active makes sense here.  There's three reasons why pgsqlConn would be NULL.  1) We never managed to connect to postgres at all _pgsql_exec().  2)  Realtime attempted to initialize, realtime*pgsql()/update*pgsql()/etc and couldn't... and then dumped the handle 3)  Realtime initialized successfully parse_config() and then dumped the handle, so we'll need to connect before the next query.

Upon discovering #3... It seems wasteful to disconnect after a successful initialization in parse_config().  Connecting to postgres is one of the more expensive parts of the query process, it would seem beneficial to keep the connection open.


- kobaz


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


On 2010-09-28 15:46:25, kobaz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/944/
> -----------------------------------------------------------
> 
> (Updated 2010-09-28 15:46:25)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> If your postgres connection died suddenly in between res_config_pgsql queries, the query will fail because the query is executed on a disconnected/disconnecting handle.
> 
> Reviewboard patch adds support for reconnecting and retrying the failed query if the connection was closed while executing the query. The patch also removes lots of duplication of checking for handling query errors.
> 
> Ideally there should be a centralized res module for postgres connections that cdr_pgsql and res_config_pgsql (and any other pgsql dependent module) can use for a centralized db abstraction module.
> 
> But I believe fixing up res_config_pgsql is the first step.
> 
> 
> Diffs
> -----
> 
>   trunk/res/res_config_pgsql.c 289174 
> 
> Diff: https://reviewboard.asterisk.org/r/944/diff
> 
> 
> Testing
> -------
> 
> res_pgsql_conf.
> ===============
> [general]
> dbhost=localhost
> dbport=5432
> dbname=pbx
> dbuser=root
> dbpass=password
> 
> 
> extconfig.conf
> ===================
> [settings]
> voicemail => pgsql,pbx,asterisk.v_asterisk_voicemail_config
> 
> ===================
> 
> Start asterisk
> (kill all current connections, replace pbx with appropriate database name)
> kill `ps aux | grep postgres | grep pbx | awk {'print $2'}`
> 
> Launch voicemail (or something else that uses res_config_pgsql)
> 
> [Sep 28 16:02:31] ERROR[23650]: res_config_pgsql.c:167 _pgsql_exec: PostgreSQL RealTime: Failed to query 'asterisk.v_asterisk_voicemail_config at pbx'.
> [Sep 28 16:02:31] ERROR[23650]: res_config_pgsql.c:168 _pgsql_exec: PostgreSQL RealTime: Query Failed: SELECT * FROM asterisk.v_asterisk_voicemail_config WHERE mailbox = '2702' AND context = 'internal'
> [Sep 28 16:02:31] ERROR[23650]: res_config_pgsql.c:169 _pgsql_exec: PostgreSQL RealTime: Query Failed because: FATAL:  terminating connection due to administrator command
> server closed the connection unexpectedly
>         This probably means the server terminated abnormally
>         before or while processing the request.
>  (PGRES_FATAL_ERROR)
> [Sep 28 16:02:31] NOTICE[23650]: res_config_pgsql.c:230 pgsql_exec: PostgreSQL RealTime: Query finally succeeded: SELECT * FROM asterisk.v_asterisk_voicemail_config WHERE mailbox = '2702' AND context = 'internal'
>     -- <SIP/2701-00000005> Playing 'vm-theperson.ulaw' (language 'en')
>     -- <SIP/2701-00000005> Playing 'digits/2.ulaw' (language 'en')
>     -- <SIP/2701-00000005> Playing 'digits/7.ulaw' (language 'en')
>     -- <SIP/2701-00000005> Playing 'digits/0.ulaw' (language 'en')
>     -- <SIP/2701-00000005> Playing 'digits/2.ulaw' (language 'en')
> 
> 
> Thanks,
> 
> kobaz
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20101225/4f2b2480/attachment.htm>