Re: elog() patch - Mailing list pgsql-hackers

From Tom Lane
Subject Re: elog() patch
Date
Msg-id 12504.1015197667@sss.pgh.pa.us
Whole thread Raw
In response to Re: elog() patch  (Bruce Momjian <pgman@candle.pha.pa.us>)
List pgsql-hackers
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I assume with your changes that the password will no longer be echoed to
> the client on failure at debug5, right?

Nope.

I've noticed a bunch of breakage with this patch at high (or is it low?)
debug output levels; for example client disconnection leaves this in the
log:DEBUG:  proc_exit(0)DEBUG:  shmem_exit(0)LOG:  pq_flush: send() failed: Broken pipeDEBUG:  exit(0)LOG:  pq_flush:
send()failed: Bad file numberDEBUG:  reaping dead processesDEBUG:  child process (pid 12462) exited with exit code 0
 
The problem is that elog is still trying to send stuff to the client
after client disconnection.  I propose to fix that by resetting
whereToSendOutput to None as soon as we detect client disconnect.

A more serious problem has to do with error reports for communication
problems, for example this fragment in pqcomm.c:
           /*            * Careful: an elog() that tries to write to the client            * would cause recursion to
here,leading to stack overflow            * and core dump!  This message must go *only* to the postmaster            *
log. elog(LOG) is presently safe.            */           elog(LOG, "pq_recvbuf: recv() failed: %m");
 

elog(LOG) is NOT safe anymore :-(.

I am thinking of inventing an additional elog level, perhaps called
COMMERR, to be used specifically for reports of client communication
trouble.  This could be treated the same as LOG as far as output to
the server log goes, but we would hard-wire it to never be reported
to the client (for fear of recursive failure).

Comments?

BTW, I am also looking at normalizing all the backend/libpq reports
that look like    snprintf(PQerrormsg, ...);    fputs(PQerrormsg, stderr);    pqdebug("%s", PQerrormsg);
to just use elog.  As-is this code is fairly broken since it doesn't
honor the syslog option.  Any objections?
        regards, tom lane


pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: elog() patch
Next
From: Tom Lane
Date:
Subject: Re: elog() patch