Re: termination of backend waiting for sync rep generates a junk log message

On Tue, Oct 18, 2011 at 5:18 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Mon, Oct 17, 2011 at 6:53 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>> The simple fix is to change InteractiveBackend() so that it calls
>>> CHECK_FOR_INTERRUPTS() before it outputs "backend> ". Thought?
>
>> I'm tempted to say we should do that in PostgresMain() instead, maybe
>> something like this:
>
>> diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
>> index 976a832..9e5557c 100644
>> --- a/src/backend/tcop/postgres.c
>> +++ b/src/backend/tcop/postgres.c
>> @@ -3769,6 +3769,8 @@ PostgresMain(int argc, char *argv[], const char *username)
>>                 MemoryContextSwitchTo(MessageContext);
>>                 MemoryContextResetAndDeleteChildren(MessageContext);
>
>> +               CHECK_FOR_INTERRUPTS();
>> +
>>                 initStringInfo(&input_message);
>
>>                 /*
>
> I don't like putting a CHECK_FOR_INTERRUPTS there, because it's way too
> late to throw an error for the previous query.
>
> The real problem here is probably that we're overloading the meaning of
> whereToSendOutput.  The reset of that variable during shutdown was only
> ever meant to prevent output from being sent to a no-longer-present
> client.  It should *not* result in trying to read a query from stdin.

I think you're right, but am not sure how to fix it.

> 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.
*/

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company