sql module, postgresql driver
hello, in case of a fatal error that occurs on the postgresql database server the sql accounting module returns "ok" so the "redundancy" feature of freeradius does not work. in file src/modules/rlm_sql/drivers/rlm_sql_postgresql/sql_postgresql.c, function sql_check_error I see that for non "SQL_DOWN" errors it returns 0, should it be -1 ? a non zero value ? thanks, Razvan Radu ::: radiusd -X ::: ====================================================== rlm_sql (sql): Reserving sql socket id: 4 rlm_sql_postgresql: Status: PGRES_FATAL_ERROR rlm_sql_postgresql: Error relation "bbb" does not exist rlm_sql_postgresql: Postgresql Fatal Error: [42P01] Accured!! rlm_sql_postgresql: Postgresql Fatal Error: [UNDEFINED TABLE] Occured!! rlm_sql (sql): Released sql socket id: 4 modcall[accounting]: module "sql" returns ok for request 351 modcall: group returns ok for request 351 modcall: group accounting returns ok for request 351 ======================================================
On Tue 02 Jan 2007 00:41, FreeRadius Devel wrote:
hello,
in case of a fatal error that occurs on the postgresql database server the sql accounting module returns "ok" so the "redundancy" feature of freeradius does not work. in file src/modules/rlm_sql/drivers/rlm_sql_postgresql/sql_postgresql.c, function sql_check_error I see that for non "SQL_DOWN" errors it returns 0, should it be -1 ? a non zero value ?
thanks, Razvan Radu
::: radiusd -X :::
====================================================== rlm_sql (sql): Reserving sql socket id: 4 rlm_sql_postgresql: Status: PGRES_FATAL_ERROR rlm_sql_postgresql: Error relation "bbb" does not exist rlm_sql_postgresql: Postgresql Fatal Error: [42P01] Accured!! rlm_sql_postgresql: Postgresql Fatal Error: [UNDEFINED TABLE] Occured!! rlm_sql (sql): Released sql socket id: 4 modcall[accounting]: module "sql" returns ok for request 351 modcall: group returns ok for request 351 modcall: group accounting returns ok for request 351 ======================================================
We have in-fact made allot of changes to the Postgres driver in CVS Head to make it much more fine grained in its Error reporting and return codes. We appreciate you testing it and finding error conditions (And spelling mistakes :-) in the code. The idea was to make it only return SQL_DOWN on errors that were indicating that the DB has disappeared intead of on all fatal errors. This required checking the Error Sub-code and some logic changes. It appears that we successfully reached our goal of making SQL_DOWN "more correct"but in the mean time made some of the other return codes "less correct". I have fixed the spelling mistakes ("Occured" and "Accured" should both be "Occurred") but the code logic will have to wait until I get back to the office in a couple of days. I didn't actually write this section of the code, one of my programmers did, and while it works for us in production, it seems that he didn't actually think through the logic very well.. Regards -- Peter Nixon http://www.peternixon.net/ PGP Key: http://www.peternixon.net/public.asc
On Tue 02 Jan 2007 00:41, FreeRadius Devel wrote:
hello,
in case of a fatal error that occurs on the postgresql database server the sql accounting module returns "ok" so the "redundancy" feature of freeradius does not work. in file src/modules/rlm_sql/drivers/rlm_sql_postgresql/sql_postgresql.c, function sql_check_error I see that for non "SQL_DOWN" errors it returns 0, should it be -1 ? a non zero value ?
You Are correct. Fixed now. Look out for a further monster (60+ line) patch to sql_postgresql.c in the next few hours. I am just putting it through testing at the moment. It fixes some other edge cases and a memory leak reported in Bug 413 Cheers -- Peter Nixon http://www.peternixon.net/ PGP Key: http://www.peternixon.net/public.asc
participants (2)
-
FreeRadius Devel -
Peter Nixon