From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: termination of backend waiting for sync rep generates a junk log message |
Date: | 2011-10-19 03:27:42 |
Message-ID: | 2619.1318994862@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Oct 18, 2011 at 5:18 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Another question worth asking is how is it that we're getting to
>> ReadCommand at all, if we have already determined that the client is
>> gone. Fixing that with an additional CHECK_FOR_INTERRUPTS seems like
>> a crock.
> We haven't determined the client is gone; we're trying to close the
> connection "unexpectedly". As the comment in SyncRepWaitForLSN
> explains:
> /*
> * If a wait for synchronous replication is pending,
> we can neither
> * acknowledge the commit nor raise ERROR or FATAL.
> The latter would
> * lead the client to believe that that the
> transaction aborted, which
> * is not true: it's already committed locally. The
> former is no good
> * either: the client has requested synchronous
> replication, and is
> * entitled to assume that an acknowledged commit is
> also replicated,
> * which might not be true. So in this case we issue a
> WARNING (which
> * some clients may be able to interpret) and shut off
> further output.
> * We do NOT reset ProcDiePending, so that the process
> will die after
> * the commit is cleaned up.
> */
Hmm. Maybe the real answer is "this code is abusing whereToSendOutput"
(and about six other things besides). I'll try to think of a better
solution, but not tonight.
One thing worth asking is why we're willing to violate half a dozen
different coding rules if we see ProcDiePending, yet we're perfectly
happy to rely on the client understanding a WARNING for the
QueryCancelPending case. Another is whether this whole function isn't
complete BS in the first place, since it appears to be coded on the
obviously-false assumption that nothing it calls can throw elog(ERROR)
--- and of course, if any of those functions do throw ERROR, all the
argumentation here goes out the window.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2011-10-19 03:45:48 | Re: new compiler warnings |
Previous Message | Tom Lane | 2011-10-19 03:08:04 | Re: [v9.2] Fix Leaky View Problem |