Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Oct 18, 2011 at 5:18 PM, Tom Lane <tgl@sss.pgh.pa.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