Redundant SQL servers accounting problem, FreeRadius 1.1.4

[Alexander V. Klepikov]

Hello, Alan!
You wrote  on Tue, 20 Mar 2007 12:47:01 +0100:

 AD> Alexander V. Klepikov wrote:

>> I applied the patch and it does not work. It seemes to me, it's
>> becuase
>> SQL socket may be unconnected and sqlsocket->conn != NULL,

AD>   That sounds like a bug to me.

It seemes to me I begin to understand, what is going on in the module 
rlm_sql_postgresql, but it is very difficult to me to write my conclusions 
in english.

I'm afraid this is not a bug. I looked in the sources, and I found that in 
module rlm_sql_postgresql in all functions is used construction

rlm_sql_postgres_sock *pg_sock = sqlsocket->conn;

Then all calls to libpq (the real PostgreSQL driver) deal with 
pg_sock->conn. Here is one of the best illustrations, function 
sql_init_socket :

=========Beginning of the citation==============
static int sql_init_socket(SQLSOCK *sqlsocket, SQL_CONFIG *config) {
        char connstring[2048];
        char *port, *host;
        rlm_sql_postgres_sock *pg_sock;

        if (config->sql_server[0] != '\0') {
                host = " host=";
        } else {
                host = "";
        }

        if (config->sql_port[0] != '\0') {
                port = " port=";
        } else {
                port = "";
        }

        if (!sqlsocket->conn) {
                sqlsocket->conn = (rlm_sql_postgres_sock 
*)rad_malloc(sizeof(rlm_sql_postgres_sock));
                if (!sqlsocket->conn) {
                        return -1;
                }
        }

        pg_sock = sqlsocket->conn;
        memset(pg_sock, 0, sizeof(*pg_sock));

        snprintf(connstring, sizeof(connstring),
                        "dbname=%s%s%s%s%s user=%s password=%s",
                        config->sql_db, host, config->sql_server,
                        port, config->sql_port,
                        config->sql_login, config->sql_password);
        pg_sock->row=NULL;
        pg_sock->result=NULL;
        pg_sock->conn=PQconnectdb(connstring);

        if (PQstatus(pg_sock->conn) == CONNECTION_BAD) {
                radlog(L_ERR, "rlm_sql_postgresql: Couldn't connect socket 
to PostgreSQL server %s@%s:%s", config->sql_login, co
                radlog(L_ERR, "rlm_sql_postgresql: Postgresql error '%s'", 
PQerrorMessage(pg_sock->conn));
                PQfinish(pg_sock->conn);
                return SQL_DOWN;
        }

        return 0;
}

=========The end of the citation================

You see, first sqlsocket->conn is inited and all database parameters are 
set.Then a connection attempt is made: pg_sock->conn=PQconnectdb(connstring) 
. If connection to DB fails, PQfinish(pg_sock->conn) is called, which frees 
pg_sock->conn - need to do this is described in libpq docs. So even in case 
of unsuccessefull connection we have good "database handle" sqlsocket->conn, 
which should not be NULL. When FreeRadius starts, sql_init_socketpool is 
called. It inits all SQL sockets and attempts to connect to database(s).
I did not find any information about what is going on when database or SQL 
server suddenly comes down, but it looks like pg_sock->conn is freed when 
connection to DB became broken. And pg_sock->conn != NULL . That's why libpq 
crashes when PQfinish(pg_sock->conn) in sql_close function is called. As far 
I understand, this is expected behavior.
According to this, I can make a conclusion that when database handle is 
checked for connectivity (in rlm_sql module), sqlsocket->state should be 
used. In theory, sqlsocket->state can equals to sockconnected when actually 
it is disconnected. It seemes to me, actually this can happen very rarely. 
May be, state of connection should be checked before running every SQL query 
to minimize risk of operation on disconnected DB, but I believe it's not 
necessary yet. Besides, it will require to modify all sql drivers.
I think there is few places left in rlm_sql module where sqlsocket->conn 
should be replaced with sqlsocket->state. I'm sure I can find and patch 
them.

With best regards, Alexander V. Klepikov.  E-mail: klepikov_a at up.ua