Hello Hackers,
After reading up through archives on the two $subj related TODO items
I'm under impression that the patches[1,2] didn't make it mainly because
of the risk of breaking SSL internals if we try to longjump out of the
signal handler in the middle of a blocking SSL read and/or if we try to
report that final "transaction/connection aborted" notice to the client.
What if instead of trying to escape from the signal handler we would set
a flag and use it to simulate EOF after the recv() is restarted due to
EINTR? From the backend perspective it should look like the client has
just hang up.
Both SSL and cleartext connections are routed through secure_raw_read,
so it seems like a good candidate for checking the flag we would set in
StatementCancelHandler(). (Should we also add the same check in
interactive_getc()?)
Ultimately, in PostgresMain() the ReadCommand() would return EOF and we
can either let the existing code shutdown the backend, or add special
handling that will only abort the transaction and report the case to the
client, which was the intent in[1]. And if that works out, the timeout
handler in[2] can simply reuse the flag.
A potential problem with this is that SSL might perform some eager
cleanup when seeing EOF, so the backend might need to reset/restart the
connection (is that possible?)
I'm attaching a patch (that doesn't compile yet :-p) in hope it can be
useful for the purpose of the discussion. Notably, secure_raw_read()
can't know about IdleTransactionCancelPending and I'm not sure what
would be the best way to let it see that (is it too much of a shortcut
anyway?)
--
Alex
[1] http://www.postgresql.org/message-id/1262268211.19367.10736.camel@ebony
[2] http://www.postgresql.org/message-id/538dc843.2070...@dalibo.com
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
new file mode 100644
index b05364c..377984d
*** a/src/backend/libpq/be-secure-openssl.c
--- b/src/backend/libpq/be-secure-openssl.c
*************** rloop:
*** 543,550 ****
errmsg("SSL error: %s", SSLerrmessage())));
/* fall through */
case SSL_ERROR_ZERO_RETURN:
! errno = ECONNRESET;
! n = -1;
break;
default:
ereport(COMMERROR,
--- 543,553 ----
errmsg("SSL error: %s", SSLerrmessage())));
/* fall through */
case SSL_ERROR_ZERO_RETURN:
! if (!IdleTransactionCancelPending) /* HACK */
! {
! errno = ECONNRESET;
! n = -1;
! }
break;
default:
ereport(COMMERROR,
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
new file mode 100644
index 41ec1ad..44079ff
*** a/src/backend/libpq/be-secure.c
--- b/src/backend/libpq/be-secure.c
*************** secure_raw_read(Port *port, void *ptr, s
*** 145,155 ****
{
ssize_t n;
! prepare_for_client_read();
! n = recv(port->sock, ptr, len, 0);
! client_read_ended();
return n;
}
--- 145,160 ----
{
ssize_t n;
! if (IdleTransactionCancelPending)
! n = 0; /* simulate EOF */
! else
! {
! prepare_for_client_read();
! n = recv(port->sock, ptr, len, 0);
! client_read_ended();
! }
return n;
}
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
new file mode 100644
index cc62b2c..37437fc
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
*************** interactive_getc(void)
*** 310,318 ****
{
int c;
! prepare_for_client_read();
! c = getc(stdin);
! client_read_ended();
return c;
}
--- 310,323 ----
{
int c;
! if (IdleTransactionCancelPending)
! c = EOF;
! else
! {
! prepare_for_client_read();
! c = getc(stdin);
! client_read_ended();
! }
return c;
}
*************** StatementCancelHandler(SIGNAL_ARGS)
*** 2629,2642 ****
if (!proc_exit_inprogress)
{
InterruptPending = true;
! QueryCancelPending = true;
/*
* If it's safe to interrupt, and we're waiting for input or a lock,
* service the interrupt immediately
*/
if (ImmediateInterruptOK && InterruptHoldoffCount == 0 &&
! CritSectionCount == 0)
{
/* bump holdoff count to make ProcessInterrupts() a no-op */
/* until we are done getting ready for it */
--- 2634,2655 ----
if (!proc_exit_inprogress)
{
InterruptPending = true;
!
! if (!DoingCommandRead)
! QueryCancelPending = true;
! else if (IsTransactionOrTransactionBlock())
! IdleTransactionCancelPending = true;
/*
* If it's safe to interrupt, and we're waiting for input or a lock,
* service the interrupt immediately
*/
if (ImmediateInterruptOK && InterruptHoldoffCount == 0 &&
! CritSectionCount == 0 && QueryCancelPending)
! /*
! * ProcessInterrupts() does nothing in case of
! * IdleTransactionCancelPending, it is handled in PostgresMain
! */
{
/* bump holdoff count to make ProcessInterrupts() a no-op */
/* until we are done getting ready for it */
*************** PostgresMain(int argc, char *argv[],
*** 4216,4221 ****
--- 4229,4247 ----
*/
case 'X':
case EOF:
+ if (IdleTransactionCancelPending)
+ {
+ IdleTransactionCancelPending = false;
+
+ ereport(NOTICE,
+ (errcode(ERRCODE_QUERY_CANCELED),
+ errmsg("canceling idle transaction due to administrator request")));
+
+ /*AbortTransactionAndAnySubtransactions();*/
+ /* ... */
+
+ break; /* live on */
+ }
/*
* Reset whereToSendOutput to prevent ereport from attempting
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
new file mode 100644
index be74835..fba92b2
*** a/src/backend/utils/init/globals.c
--- b/src/backend/utils/init/globals.c
*************** ProtocolVersion FrontendProtocol;
*** 28,33 ****
--- 28,34 ----
volatile bool InterruptPending = false;
volatile bool QueryCancelPending = false;
+ volatile bool IdleTransactionCancelPending = false;
volatile bool ProcDiePending = false;
volatile bool ClientConnectionLost = false;
volatile bool ImmediateInterruptOK = false;
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
new file mode 100644
index 1558a75..ec1047c
*** a/src/include/miscadmin.h
--- b/src/include/miscadmin.h
***************
*** 75,80 ****
--- 75,81 ----
/* these are marked volatile because they are set by signal handlers: */
extern PGDLLIMPORT volatile bool InterruptPending;
extern PGDLLIMPORT volatile bool QueryCancelPending;
+ extern PGDLLIMPORT volatile bool IdleTransactionCancelPending;
extern PGDLLIMPORT volatile bool ProcDiePending;
extern volatile bool ClientConnectionLost;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
