From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Kevin Grittner <kgrittn(at)ymail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Vik Fearing <vik(dot)fearing(at)dalibo(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Josh Berkus <josh(at)agliodbs(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net> |
Subject: | Re: idle_in_transaction_timeout |
Date: | 2014-06-29 23:31:53 |
Message-ID: | 20140629233153.GH26930@awork2.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2014-06-29 19:13:55 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-06-29 12:53:56 -0400, Tom Lane wrote:
> >> I do not think it is: specifically, the notion
> >> that we will call ereport(FATAL) directly from a signal handler
> >> does not fill me with warm fuzzies.
>
> > Aren't we already pretty much doing that for
> > SIGTERM/pg_terminate_backend() and recovery conflict interrupts?
>
> We do that *only at safe spots*, ie CHECK_FOR_INTERRUPTS. Or at least
> I sure hope so. Otherwise interrupting, eg, malloc will lead to much
> unhappiness.
I was specifically thinking about us immediately reacting to those while
we're reading from the client. We're indeed doing that directly:
#1 0x000000000076648a in proc_exit (code=1) at /home/andres/src/postgresql/src/backend/storage/ipc/ipc.c:143
#2 0x00000000008bcbf7 in errfinish (dummy=0) at /home/andres/src/postgresql/src/backend/utils/error/elog.c:555
#3 0x00000000007909f7 in ProcessInterrupts () at /home/andres/src/postgresql/src/backend/tcop/postgres.c:2869
#4 0x0000000000790469 in die (postgres_signal_arg=15) at /home/andres/src/postgresql/src/backend/tcop/postgres.c:2604
#5 <signal handler called>
#6 0x00007fb7c8ca0ebb in __libc_recv (fd=10, buf=0xd5f240 <PqRecvBuffer>, n=8192, flags=-1) at ../sysdeps/unix/sysv/linux/x86_64/recv.c:29
#7 0x000000000066a07c in secure_read (port=0x1a29d30, ptr=0xd5f240 <PqRecvBuffer>, len=8192)
at /home/andres/src/postgresql/src/backend/libpq/be-secure.c:317
#8 0x00000000006770b5 in pq_recvbuf () at /home/andres/src/postgresql/src/backend/libpq/pqcomm.c:854
#9 0x000000000067714f in pq_getbyte () at /home/andres/src/postgresql/src/backend/libpq/pqcomm.c:895
#10 0x000000000078d26b in SocketBackend (inBuf=0x7fffbc02bc30) at /home/andres/src/postgresql/src/backend/tcop/postgres.c:335
#11 0x000000000078d659 in ReadCommand (inBuf=0x7fffbc02bc30) at /home/andres/src/postgresql/src/backend/tcop/postgres.c:483
Note that we're *inside* recv() here. Both paths to recv, without ssl
and with, have called prepare_for_client_read() which sets
ImmediateInterruptOK = true. Right now I fail to see why it's safe to do
so, at least when inside openssl.
> > BUT: why is what ProcessInterrupts() is doing safe? Shouldn't it always
> > at least set whereToSendOutput = DestNone when FATALing while reading
> > (potentially via openssl)?
>
> Uh, no, that would pretty much destroy the point of trying to report
> an error message at all.
I only mean that we should do so in scenarios where we're currently
reading from the client. For die(), while we're reading from the client,
we're sending a message the client doesn't expect - and thus
unsurprisingly doesn't report. The client will just report 'server
closed the connection unexpectedly'.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2014-06-29 23:51:05 | Re: idle_in_transaction_timeout |
Previous Message | Tomas Vondra | 2014-06-29 23:24:44 | Re: bad estimation together with large work_mem generates terrible slow hash joins |