smart shutdown at end of transaction (was: Default mode for shutdown)

Started by Robert Haasover 13 years ago45 messages
#1Robert Haas
robertmhaas@gmail.com
1 attachment(s)

On Wed, Dec 15, 2010 at 10:11 AM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

It occurs to me that we may need a new mode, which disconnects sessions
that are not in a transaction (or as soon as they are) but leaves
in-progress transactions alone; this could be the new default.  Of
course, this is much more difficult to implement than the current modes.

This idea appeared to have some support. I'd like to suggest that we
take this a step further. Instead of adding a fourth mode, I'd like
to suggest that we redefine "smart" to have the behavior described
above. This is based on the theory that (1) people who like smart
shutdown like it because it allows currently-running transactions to
complete without error, and will find it acceptable to have idle
transactions terminated immediately and other sessions terminated
after the command completes; and (2) people who dislike smart shutdown
(such as me) dislike it primarily because a completely idle session
that someone's forgotten to close can prevent shutdown indefinitely.
Either part of this theory could be wrong, of course, although I'm
pretty sure #2 holds for me personally at the least.

Patch is attached.

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

Attachments:

smart-shutdown-at-eoxact.patchapplication/octet-stream; name=smart-shutdown-at-eoxact.patchDownload
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 5785450..66b658e 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -1413,8 +1413,11 @@ echo -17 > /proc/self/oom_adj
       <para>
        This is the <firstterm>Smart Shutdown</firstterm> mode.
        After receiving <systemitem>SIGTERM</systemitem>, the server
-       disallows new connections, but lets existing sessions end their
-       work normally. It shuts down only after all of the sessions terminate.
+       disallows new connections and sends all existing server processes
+       <systemitem>SIGUSR2</systemitem>, which will cause them
+       to exit once any transaction currently in progress ends, or
+       at once if no transaction is currently in progress.
+       It shuts down only after all of the sessions terminate.
        If the server is in online backup mode, it additionally waits
        until online backup mode is no longer active.  While backup mode is
        active, new connections will still be allowed, but only to superusers
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index d59ee68..08768ba 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2126,6 +2126,8 @@ pmdie(SIGNAL_ARGS)
 			if (pmState == PM_RUN || pmState == PM_RECOVERY ||
 				pmState == PM_HOT_STANDBY || pmState == PM_STARTUP)
 			{
+				/* normal children are told to shut down at end of txn */
+				SignalSomeChildren(SIGUSR2, BACKEND_TYPE_NORMAL);
 				/* autovacuum workers are told to shut down immediately */
 				SignalSomeChildren(SIGTERM, BACKEND_TYPE_AUTOVAC);
 				/* and the autovac launcher too */
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 51d623f..75ef141 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2551,7 +2551,32 @@ quickdie(SIGNAL_ARGS)
 }
 
 /*
- * Shutdown signal from postmaster: abort transaction and exit
+ * Smart shutdown signal from postmaster: exit if or when no transaction is
+ * in progress
+ */
+static void
+smart_shutdown_handler(SIGNAL_ARGS)
+{
+	int			save_errno = errno;
+
+	/* Don't joggle the elbow of proc_exit */
+	if (!proc_exit_inprogress)
+	{
+		SmartShutdownPending = true;
+		if (IdleOutsideTransaction && SmartShutdownPending && DoingCommandRead)
+		{
+			IdleOutsideTransaction = false;
+			ereport(FATAL,
+					(errcode(ERRCODE_ADMIN_SHUTDOWN),
+			 errmsg("terminating connection due to administrator command")));
+		}
+	}
+
+	errno = save_errno;
+}
+
+/*
+ * Fast shutdown signal from postmaster: abort transaction and exit
  * at soonest convenient time
  */
 void
@@ -3595,7 +3620,7 @@ PostgresMain(int argc, char *argv[], const char *username)
 		 */
 		pqsignal(SIGPIPE, SIG_IGN);
 		pqsignal(SIGUSR1, procsignal_sigusr1_handler);
-		pqsignal(SIGUSR2, SIG_IGN);
+		pqsignal(SIGUSR2, smart_shutdown_handler);
 		pqsignal(SIGFPE, FloatExceptionHandler);
 
 		/*
@@ -3883,6 +3908,7 @@ PostgresMain(int argc, char *argv[], const char *username)
 
 				set_ps_display("idle", false);
 				pgstat_report_activity(STATE_IDLE, NULL);
+				IdleOutsideTransaction = true;
 			}
 
 			ReadyForQuery(whereToSendOutput);
@@ -3898,17 +3924,35 @@ PostgresMain(int argc, char *argv[], const char *username)
 		DoingCommandRead = true;
 
 		/*
-		 * (3) read a command (loop blocks here)
+		 * (3) If we're idle and not in a transaction, and a smart shutdown
+		 * has been requested, exit gracefully.
+		 */
+		if (IdleOutsideTransaction && SmartShutdownPending)
+		{
+			DoingCommandRead = false;
+			IdleOutsideTransaction = false;
+			ereport(FATAL,
+					(errcode(ERRCODE_ADMIN_SHUTDOWN),
+			 errmsg("terminating connection due to administrator command")));
+		}
+
+		/*
+		 * (4) read a command (loop blocks here)
 		 */
 		firstchar = ReadCommand(&input_message);
 
 		/*
-		 * (4) disable async signal conditions again.
+		 * (5) got a command, so no longer idle
+		 */
+		IdleOutsideTransaction = false;
+
+		/*
+		 * (6) disable async signal conditions again.
 		 */
 		DoingCommandRead = false;
 
 		/*
-		 * (5) check for any other interesting events that happened while we
+		 * (7) check for any other interesting events that happened while we
 		 * slept.
 		 */
 		if (got_SIGHUP)
@@ -3918,7 +3962,7 @@ PostgresMain(int argc, char *argv[], const char *username)
 		}
 
 		/*
-		 * (6) process the command.  But ignore it if we're skipping till
+		 * (8) process the command.  But ignore it if we're skipping till
 		 * Sync.
 		 */
 		if (ignore_till_sync && firstchar != EOF)
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 4b66bd3..c48eb80 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -26,11 +26,13 @@
 
 ProtocolVersion FrontendProtocol;
 
+volatile bool SmartShutdownPending = false;
 volatile bool InterruptPending = false;
 volatile bool QueryCancelPending = false;
 volatile bool ProcDiePending = false;
 volatile bool ClientConnectionLost = false;
 volatile bool ImmediateInterruptOK = false;
+volatile bool IdleOutsideTransaction = false;
 volatile uint32 InterruptHoldoffCount = 0;
 volatile uint32 CritSectionCount = 0;
 
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index b186eed..203c2b7 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -72,6 +72,7 @@
 /* in globals.c */
 /* these are marked volatile because they are set by signal handlers: */
 extern PGDLLIMPORT volatile bool InterruptPending;
+extern volatile bool SmartShutdownPending;
 extern volatile bool QueryCancelPending;
 extern volatile bool ProcDiePending;
 
@@ -79,6 +80,7 @@ extern volatile bool ClientConnectionLost;
 
 /* these are marked volatile because they are examined by signal handlers: */
 extern volatile bool ImmediateInterruptOK;
+extern volatile bool IdleOutsideTransaction;
 extern PGDLLIMPORT volatile uint32 InterruptHoldoffCount;
 extern PGDLLIMPORT volatile uint32 CritSectionCount;
 
#2Magnus Hagander
magnus@hagander.net
In reply to: Robert Haas (#1)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Fri, Apr 27, 2012 at 19:42, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Dec 15, 2010 at 10:11 AM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

It occurs to me that we may need a new mode, which disconnects sessions
that are not in a transaction (or as soon as they are) but leaves
in-progress transactions alone; this could be the new default.  Of
course, this is much more difficult to implement than the current modes.

This idea appeared to have some support.  I'd like to suggest that we
take this a step further.  Instead of adding a fourth mode, I'd like
to suggest that we redefine "smart" to have the behavior described

+1762329!

above.  This is based on the theory that (1) people who like smart
shutdown like it because it allows currently-running transactions to
complete without error, and will find it acceptable to have idle
transactions terminated immediately and other sessions terminated

Uh, I don't think it's ok to terminate an idle transaction
immediately. An idle *session* is ok, though - maybe that's what you
mean?

Because every transaction that's *doing* multiple things will be idle
for milliseconds every now and then.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#3Robert Haas
robertmhaas@gmail.com
In reply to: Magnus Hagander (#2)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Fri, Apr 27, 2012 at 1:46 PM, Magnus Hagander <magnus@hagander.net> wrote:

On Fri, Apr 27, 2012 at 19:42, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Dec 15, 2010 at 10:11 AM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

It occurs to me that we may need a new mode, which disconnects sessions
that are not in a transaction (or as soon as they are) but leaves
in-progress transactions alone; this could be the new default.  Of
course, this is much more difficult to implement than the current modes.

This idea appeared to have some support.  I'd like to suggest that we
take this a step further.  Instead of adding a fourth mode, I'd like
to suggest that we redefine "smart" to have the behavior described

+1762329!

Thanks. :-)

above.  This is based on the theory that (1) people who like smart
shutdown like it because it allows currently-running transactions to
complete without error, and will find it acceptable to have idle
transactions terminated immediately and other sessions terminated

Uh, I don't think it's ok to terminate an idle transaction
immediately. An idle *session* is ok, though - maybe that's what you
mean?

Yes, exactly. What the patch does is arrange things so that, when
smart shutdown is requested, we terminate each session as soon as it
is both (1) idle and (2) not in a transaction.

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#1)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Dec 15, 2010 at 10:11 AM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

It occurs to me that we may need a new mode, which disconnects sessions
that are not in a transaction (or as soon as they are) but leaves
in-progress transactions alone; this could be the new default. Of
course, this is much more difficult to implement than the current modes.

This idea appeared to have some support. I'd like to suggest that we
take this a step further. Instead of adding a fourth mode, I'd like
to suggest that we redefine "smart" to have the behavior described
above.

No, I'm not happy with that. Smart shutdown is defined to not affect
current sessions. I'm fine with having a fourth mode that acts as you
suggest (and, probably, even with making it the default); but not with
taking away a behavior that people may well be relying on.

This is based on the theory that (1) people who like smart
shutdown like it because it allows currently-running transactions to
complete without error,

I think they like it because it allows currently-running *sessions*
to complete without error. You have no real basis for asserting that
relocating that goalpost won't change the game.

regards, tom lane

#5Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#4)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Fri, Apr 27, 2012 at 7:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Dec 15, 2010 at 10:11 AM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

It occurs to me that we may need a new mode, which disconnects sessions
that are not in a transaction (or as soon as they are) but leaves
in-progress transactions alone; this could be the new default.  Of
course, this is much more difficult to implement than the current modes.

This idea appeared to have some support.  I'd like to suggest that we
take this a step further.  Instead of adding a fourth mode, I'd like
to suggest that we redefine "smart" to have the behavior described
above.

No, I'm not happy with that.  Smart shutdown is defined to not affect
current sessions.  I'm fine with having a fourth mode that acts as you
suggest (and, probably, even with making it the default); but not with
taking away a behavior that people may well be relying on.

Agreed, but not sure what to call the new mode: "smarter"?

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#6Andres Freund
andres@anarazel.de
In reply to: Robert Haas (#1)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Hi,

On Friday, April 27, 2012 07:42:59 PM Robert Haas wrote:

On Wed, Dec 15, 2010 at 10:11 AM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

It occurs to me that we may need a new mode, which disconnects sessions
that are not in a transaction (or as soon as they are) but leaves
in-progress transactions alone; this could be the new default. Of
course, this is much more difficult to implement than the current modes.

This idea appeared to have some support. I'd like to suggest that we
take this a step further. Instead of adding a fourth mode, I'd like
to suggest that we redefine "smart" to have the behavior described
above. This is based on the theory that (1) people who like smart
shutdown like it because it allows currently-running transactions to
complete without error, and will find it acceptable to have idle
transactions terminated immediately and other sessions terminated
after the command completes; and (2) people who dislike smart shutdown
(such as me) dislike it primarily because a completely idle session
that someone's forgotten to close can prevent shutdown indefinitely.
Either part of this theory could be wrong, of course, although I'm
pretty sure #2 holds for me personally at the least.

I think the current smart mode is rather useful. There is quite some stuff
that you cannot do inside a transaction - or it doesn't make sense - which
still needs to shutdown gracefully. E.g. transaction managers.

Andres

#7Andres Freund
andres@anarazel.de
In reply to: Simon Riggs (#5)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Friday, April 27, 2012 08:38:10 PM Simon Riggs wrote:

On Fri, Apr 27, 2012 at 7:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Wed, Dec 15, 2010 at 10:11 AM, Alvaro Herrera

<alvherre@commandprompt.com> wrote:

It occurs to me that we may need a new mode, which disconnects sessions
that are not in a transaction (or as soon as they are) but leaves
in-progress transactions alone; this could be the new default. Of
course, this is much more difficult to implement than the current
modes.

This idea appeared to have some support. I'd like to suggest that we
take this a step further. Instead of adding a fourth mode, I'd like
to suggest that we redefine "smart" to have the behavior described
above.

No, I'm not happy with that. Smart shutdown is defined to not affect
current sessions. I'm fine with having a fourth mode that acts as you
suggest (and, probably, even with making it the default); but not with
taking away a behavior that people may well be relying on.

Agreed, but not sure what to call the new mode: "smarter"?

graceful?

Andres

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Simon Riggs (#5)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Simon Riggs <simon@2ndQuadrant.com> writes:

On Fri, Apr 27, 2012 at 7:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

No, I'm not happy with that. Smart shutdown is defined to not affect
current sessions. I'm fine with having a fourth mode that acts as you
suggest (and, probably, even with making it the default); but not with
taking away a behavior that people may well be relying on.

Agreed, but not sure what to call the new mode: "smarter"?

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior. How about

slow - allow existing sessions to finish (old "smart")
smart - allow existing transactions to finish (new)
fast - kill active queries
immediate - unclean shutdown

regards, tom lane

#9Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#8)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Fri, Apr 27, 2012 at 20:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Simon Riggs <simon@2ndQuadrant.com> writes:

On Fri, Apr 27, 2012 at 7:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

No, I'm not happy with that.  Smart shutdown is defined to not affect
current sessions.  I'm fine with having a fourth mode that acts as you
suggest (and, probably, even with making it the default); but not with
taking away a behavior that people may well be relying on.

Agreed, but not sure what to call the new mode: "smarter"?

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior.  How about

       slow    - allow existing sessions to finish (old "smart")

How about "wait" instead of "slow"?

       smart   - allow existing transactions to finish (new)

and still default, right?

       fast    - kill active queries
       immediate - unclean shutdown

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#9)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Magnus Hagander <magnus@hagander.net> writes:

On Fri, Apr 27, 2012 at 20:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior. How about

slow - allow existing sessions to finish (old "smart")

How about "wait" instead of "slow"?

I kinda liked "slow" vs "fast", but if you think that's too cute ...
("wait" doesn't seem very good, though, since all these except immediate
are waiting, just for different things.)

smart - allow existing transactions to finish (new)

and still default, right?

Right.

fast - kill active queries
immediate - unclean shutdown

regards, tom lane

#11Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#4)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Fri, Apr 27, 2012 at 2:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

This idea appeared to have some support.  I'd like to suggest that we
take this a step further.  Instead of adding a fourth mode, I'd like
to suggest that we redefine "smart" to have the behavior described
above.

No, I'm not happy with that.  Smart shutdown is defined to not affect
current sessions.  I'm fine with having a fourth mode that acts as you
suggest (and, probably, even with making it the default); but not with
taking away a behavior that people may well be relying on.

I think there is no point at all in having a discussion about this
unless we can first agree that the overwhelming majority of people who
have commented on this issue on this list are unhappy with the current
default behavior. If we are not going to change the default behavior,
then there is zero point in talking about this. So I am nervous about
your use of the word "probably", because I do not want to do a bunch
of work on this just to add a fourth shutdown mode without changing
the default to something that does not suck. I would like to get some
agreement that we ARE going to change the default behavior, and then
we can argue about what exactly we're going to change it to.

This is based on the theory that (1) people who like smart
shutdown like it because it allows currently-running transactions to
complete without error,

I think they like it because it allows currently-running *sessions*
to complete without error.  You have no real basis for asserting that
relocating that goalpost won't change the game.

I'm not asserting that. What I am asserting is that the vast majority
of users will consider the revised game to be more fun than the
original one.

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

#12Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#8)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Fri, Apr 27, 2012 at 2:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior.  How about

       slow    - allow existing sessions to finish (old "smart")
       smart   - allow existing transactions to finish (new)
       fast    - kill active queries
       immediate - unclean shutdown

I could live with that. Really, I'd like to have fast just be the
default. But the above compromise would still be a big improvement
over what we have now, assuming the new smart becomes the default.

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

#13Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Tom Lane (#10)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On 27.04.2012 21:56, Tom Lane wrote:

Magnus Hagander<magnus@hagander.net> writes:

On Fri, Apr 27, 2012 at 20:48, Tom Lane<tgl@sss.pgh.pa.us> wrote:

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior. How about

slow - allow existing sessions to finish (old "smart")

How about "wait" instead of "slow"?

I kinda liked "slow" vs "fast", but if you think that's too cute ...
("wait" doesn't seem very good, though, since all these except immediate
are waiting, just for different things.)

All the modes indeed wait (except for immediate), so I think it would
make sense to define the modes in terms of *what* they wait for.

wait sessions - allow existing sessions to finish (old "smart")
wait transactions - allow existing transactions to finish (new)
wait checkpoint - kill active queries
wait none - unclean shutdown

Hmm, the latter two are perhaps a bit confusing. So maybe:

wait_sessions - allow existing sessions to finish (old "smart")
wait_transactions - allow existing transactions to finish (new)
fast - kill active queries
immediate - unclean shutdown

Just thinking out loud here..

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

#14Robert Haas
robertmhaas@gmail.com
In reply to: Robert Haas (#12)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Fri, Apr 27, 2012 at 3:00 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Apr 27, 2012 at 2:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior.  How about

       slow    - allow existing sessions to finish (old "smart")
       smart   - allow existing transactions to finish (new)
       fast    - kill active queries
       immediate - unclean shutdown

I could live with that.  Really, I'd like to have fast just be the
default.  But the above compromise would still be a big improvement
over what we have now, assuming the new smart becomes the default.

So right now, we have a mapping from signals to shutdown types that
looks like this:

[Current] SIGTERM -> smart, SIGINT -> fast, SIGQUIT -> immediate

It seems we need another signal for the new mode, and the obvious
candidate is SIGUSR2. But what shall the mapping look like?

[Choice #1] SIGUSR2 -> slow, SIGTERM -> smart, SIGINT -> fast, SIGQUIT
-> immediate
[Choice #2] SIGTERM -> slow, SIGUSR2 -> smart, SIGINT -> fast, SIGQUIT
-> immediate

In other words, should we retain the existing behavior for SIGTERM and
make SIGUSR2 have the new behavior (choice #2)? Or shall we preserve
the invariant that SIGTERM invokes the default shutdown mode, and move
the current default behavior off into SIGUSR2 land (choice #1)?

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

#15Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Heikki Linnakangas (#13)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:

Just thinking out loud here..

In the spirit of kicking around ideas...

For those writing service scripts where you want a time limit on how
long a stop can take, so that the service script doesn't prevent OS
shutdown within a bounded time, it would also be nice to add an
escalation time limit; so if there isn't a shutdown withing k
seconds at one level it goes to the next. If the building is on
fire and you need to power down all equipment before the fire
department cuts power and starts spraying water (a situation we had
at a courthouse a year or two ago), you really don't want the OS
waiting for anything for more than a limited number of seconds
before escalating to immediate. We do that in our sh scripts now,
by using kill and sleep instead of trusting pg_ctl, but it seems
like it would be better to have pg_ctl know how to do that.

maybe?:

--escalate-after=seconds

-Kevin

#16Peter Eisentraut
peter_e@gmx.net
In reply to: Andres Freund (#6)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On fre, 2012-04-27 at 20:39 +0200, Andres Freund wrote:

I think the current smart mode is rather useful. There is quite some
stuff that you cannot do inside a transaction - or it doesn't make
sense - which still needs to shutdown gracefully. E.g. transaction
managers.

Could you elaborate on that? What would happen to the transaction
manager if you terminate any idle, not-in-a-transaction database backend
sessions it has established?

#17Andres Freund
andres@anarazel.de
In reply to: Peter Eisentraut (#16)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Friday, April 27, 2012 10:17:59 PM Peter Eisentraut wrote:

On fre, 2012-04-27 at 20:39 +0200, Andres Freund wrote:

I think the current smart mode is rather useful. There is quite some
stuff that you cannot do inside a transaction - or it doesn't make
sense - which still needs to shutdown gracefully. E.g. transaction
managers.

Could you elaborate on that? What would happen to the transaction
manager if you terminate any idle, not-in-a-transaction database backend
sessions it has established?

In the few cases where I investigated it TMs don't use transactions themselves
(which I think is correct, they don't need them), so terminating any idle
session - which the TM would appear as, as its not using txns - would leave
prepared transactions in a limbo state till the database is up again, instead
of waiting till all prepared transactions are either aborted or committed. It
may also choose to coordinate to abort all transactions, but all that is hard
if the database shuts you out.
I actually also have co-maintained other software where some processes have an
idle connection open on which some shutdown stuff will happen. Obviously all
those will need to handle the case where the connection was aborted, but that
may result in suboptimal behaviour. Requiring such processes to keep open a
transaction doesn't seem to be a good design choice in the pg world.

Andres

#18Merlin Moncure
mmoncure@gmail.com
In reply to: Robert Haas (#11)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Fri, Apr 27, 2012 at 1:57 PM, Robert Haas <robertmhaas@gmail.com> wrote:

I think there is no point at all in having a discussion about this
unless we can first agree that the overwhelming majority of people who
have commented on this issue on this list are unhappy with the current
default behavior.

count me in. the current behavior sucks.

merlin

#19Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#14)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Robert Haas <robertmhaas@gmail.com> writes:

It seems we need another signal for the new mode, and the obvious
candidate is SIGUSR2. But what shall the mapping look like?

[Choice #1] SIGUSR2 -> slow, SIGTERM -> smart, SIGINT -> fast, SIGQUIT
-> immediate
[Choice #2] SIGTERM -> slow, SIGUSR2 -> smart, SIGINT -> fast, SIGQUIT
-> immediate

SIGTERM needs to correspond to a fairly aggressive shutdown mode,
since (at least on some systems) init will send that during the system
shutdown sequence, shortly before escalating to SIGKILL. So I think
choice #2 is not sensible at all.

If we were willing to consider wholesale breakage of any scripts that
send these signals directly, I'd almost consider that it should be
SIGUSR2, SIGINT, SIGTERM, SIGQUIT. But that might be more churn than
we want. Keeping SIGTERM attached to the default/"smart" shutdown mode
seems like a reasonable compromise.

regards, tom lane

#20Peter Eisentraut
peter_e@gmx.net
In reply to: Andres Freund (#17)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On fre, 2012-04-27 at 22:30 +0200, Andres Freund wrote:

In the few cases where I investigated it TMs don't use transactions
themselves (which I think is correct, they don't need them), so
terminating any idle session - which the TM would appear as, as its
not using txns - would leave prepared transactions in a limbo state
till the database is up again, instead of waiting till all prepared
transactions are either aborted or committed. It may also choose to
coordinate to abort all transactions, but all that is hard if the
database shuts you out.

This would lead to another shutdown mode, one that terminates idle
sessions unless they have prepared transactions. That could be useful.

#21Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#19)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On fre, 2012-04-27 at 18:09 -0400, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

It seems we need another signal for the new mode, and the obvious
candidate is SIGUSR2. But what shall the mapping look like?

[Choice #1] SIGUSR2 -> slow, SIGTERM -> smart, SIGINT -> fast, SIGQUIT
-> immediate
[Choice #2] SIGTERM -> slow, SIGUSR2 -> smart, SIGINT -> fast, SIGQUIT
-> immediate

SIGTERM needs to correspond to a fairly aggressive shutdown mode,
since (at least on some systems) init will send that during the system
shutdown sequence, shortly before escalating to SIGKILL.

That only happens if the postgresql init script itself didn't do a good
job. We already have this setup currently, and it doesn't seem to cause
a great deal of problems.

If we were willing to consider wholesale breakage of any scripts that
send these signals directly, I'd almost consider that it should be
SIGUSR2, SIGINT, SIGTERM, SIGQUIT. But that might be more churn than
we want. Keeping SIGTERM attached to the default/"smart" shutdown mode
seems like a reasonable compromise.

I don't think we should change the traditional "severity" order of
signals.

#22Simon Riggs
simon@2ndQuadrant.com
In reply to: Robert Haas (#11)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Fri, Apr 27, 2012 at 7:57 PM, Robert Haas <robertmhaas@gmail.com> wrote:

I think there is no point at all in having a discussion about this
unless we can first agree that the overwhelming majority of people who
have commented on this issue on this list are unhappy with the current
default behavior.  If we are not going to change the default behavior,
then there is zero point in talking about this.

That doesn't follow.

You are right to bring up this issue. Many people do think current
"smart" mode is annoying, though we must accept that some people like
it *and* that changing the default behaviour in one release is a bad
thing.

I don't think anyone has spoken against introducing a new mode. Having
it is a good thing, whether or not it is default.

So lets implement the new shutdown mode and work out a transition path
to a new default. Changing rapidly screws up the people we love the
most.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#23Simon Riggs
simon@2ndQuadrant.com
In reply to: Heikki Linnakangas (#13)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Fri, Apr 27, 2012 at 8:36 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

All the modes indeed wait (except for immediate), so I think it would make
sense to define the modes in terms of *what* they wait for.

       wait sessions   - allow existing sessions to finish (old "smart")
       wait transactions       - allow existing transactions to finish (new)
       wait checkpoint - kill active queries
       wait none - unclean shutdown

Hmm, the latter two are perhaps a bit confusing. So maybe:

       wait_sessions   - allow existing sessions to finish (old "smart")
       wait_transactions       - allow existing transactions to finish (new)

       fast    - kill active queries
       immediate - unclean shutdown

Just thinking out loud here..

+1

Wonderfully clear, little need to check the docs to see what the terms
actually mean.

New names for both allow us to deprecate use of "smart", since it was
a silly term anyway. We keep smart for one more
release==wait_sessions, then throw an error in later releases.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#24Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#20)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Peter Eisentraut <peter_e@gmx.net> writes:

On fre, 2012-04-27 at 22:30 +0200, Andres Freund wrote:

In the few cases where I investigated it TMs don't use transactions
themselves (which I think is correct, they don't need them), so
terminating any idle session - which the TM would appear as, as its
not using txns - would leave prepared transactions in a limbo state
till the database is up again, instead of waiting till all prepared
transactions are either aborted or committed. It may also choose to
coordinate to abort all transactions, but all that is hard if the
database shuts you out.

This would lead to another shutdown mode, one that terminates idle
sessions unless they have prepared transactions. That could be useful.

Huh? Prepared transactions aren't associated with sessions. At least
not in a context using a TM --- the TM will be doing commits or
rollbacks from a session different from the ones that ran the prepared
transactions.

regards, tom lane

#25Robert Haas
robertmhaas@gmail.com
In reply to: Simon Riggs (#22)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Sat, Apr 28, 2012 at 7:04 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Fri, Apr 27, 2012 at 7:57 PM, Robert Haas <robertmhaas@gmail.com> wrote:

I think there is no point at all in having a discussion about this
unless we can first agree that the overwhelming majority of people who
have commented on this issue on this list are unhappy with the current
default behavior.  If we are not going to change the default behavior,
then there is zero point in talking about this.

That doesn't follow.

You are right to bring up this issue. Many people do think current
"smart" mode is annoying, though we must accept that some people like
it *and* that changing the default behaviour in one release is a bad
thing.

I don't think anyone has spoken against introducing a new mode. Having
it is a good thing, whether or not it is default.

So lets implement the new shutdown mode and work out a transition path
to a new default. Changing rapidly screws up the people we love the
most.

In some cases, there are ways to phase in a change over a series of
releases, but I don't see how that would be possible here. If we
intend ever to change the default mode, then we have to do it
sometime, and that release is going to have a backward-incompatibility
no matter which one it is. Personally, as backward incompatibilities
go, I think this one is pretty minor. Most people are probably
already using scripts that specify fast mode, and those scripts won't
change. But even for people who actually are using smart mode, most
people do not shut down the database all that often, and it's rather
pessimistic to suppose that the proposed new mode will break anything
for them. But even if it does, we can't make improvements to the
system without sometimes changing things in a backward-incompatible
way, and if we get into the mind-set that no amount of
backward-incompatibility is ever acceptable, we're going to seriously
limit our opportunities to revisit poor design decisions.

I think there's a funny kind of thing that happens when we discuss a
behavior that is sub-optimal: we start to look for ways to justify
leaving it the way it is, because surely it couldn't be a terrible
idea if it's been like that forever. I think there's some of that
going on on the thread about stripping trailing null columns, too: if
we've got a benchmark result showing that the patch saves CPU time on
a 5-column table (!), then all the pontificating about 700-column
tables being rare is irrelevant. Similarly here: it's true that
someone might have to revisit their init scripts, but should they fail
to do so, the consequences are really not that dire.

On the other hand, in PostgreSQL 8.4, we changed TRUNCATE to wipe out
the entire inheritance hierarchy instead of only the named table
(unless the new ONLY keyword was added). This obviously has the
potential to be completely disastrous for someone with a very
particular usage pattern, but there was little discussion and everyone
basically said "yeah, we should go ahead and change that, despite the
small risk that someone will accidentally blow away a lot more data
than they intended". Maybe there are more people using smart shutdown
than there are people truncating only the root of an inheritance
hierarchy, but nothing we're proposing to do here is going to
permanently erase anyone's data, either.

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

#26Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#24)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On lör, 2012-04-28 at 11:12 -0400, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

On fre, 2012-04-27 at 22:30 +0200, Andres Freund wrote:

In the few cases where I investigated it TMs don't use transactions
themselves (which I think is correct, they don't need them), so
terminating any idle session - which the TM would appear as, as its
not using txns - would leave prepared transactions in a limbo state
till the database is up again, instead of waiting till all prepared
transactions are either aborted or committed. It may also choose to
coordinate to abort all transactions, but all that is hard if the
database shuts you out.

This would lead to another shutdown mode, one that terminates idle
sessions unless they have prepared transactions. That could be useful.

Huh? Prepared transactions aren't associated with sessions. At least
not in a context using a TM --- the TM will be doing commits or
rollbacks from a session different from the ones that ran the prepared
transactions.

From what Andres wrote I gather that the TM would be using the same
session for preparing and committing.

In any case, if either the existing session of the TM is cut or it
cannot create a new connection, it will, after some time, have to give
up roll back the prepared transactions on the other servers. So some
kind of setting to not shut down if there are prepared transactions
pending could be useful. But this could probably be a separate GUC
setting or two instead of a shutdown mode (or two) of its own.

#27Peter Eisentraut
peter_e@gmx.net
In reply to: Robert Haas (#11)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On fre, 2012-04-27 at 14:57 -0400, Robert Haas wrote:

I think there is no point at all in having a discussion about this
unless we can first agree that the overwhelming majority of people who
have commented on this issue on this list are unhappy with the current
default behavior. If we are not going to change the default behavior,
then there is zero point in talking about this.

Have you reviewed the previous discussions where changing the default
behavior was discussed and rejected? I don't like the current default
any more than you do, but without any new arguments, there is, as you
say, zero point in talking about this.

#28Simon Riggs
simon@2ndQuadrant.com
In reply to: Robert Haas (#25)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Sun, Apr 29, 2012 at 12:41 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Sat, Apr 28, 2012 at 7:04 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

So lets implement the new shutdown mode and work out a transition path
to a new default. Changing rapidly screws up the people we love the
most.

In some cases, there are ways to phase in a change over a series of
releases, but I don't see how that would be possible here.  If we
intend ever to change the default mode, then we have to do it
sometime, and that release is going to have a backward-incompatibility
no matter which one it is.  Personally, as backward incompatibilities
go, I think this one is pretty minor.  Most people are probably
already using scripts that specify fast mode, and those scripts won't
change.  But even for people who actually are using smart mode, most
people do not shut down the database all that often, and it's rather
pessimistic to suppose that the proposed new mode will break anything
for them.  But even if it does, we can't make improvements to the
system without sometimes changing things in a backward-incompatible
way, and if we get into the mind-set that no amount of
backward-incompatibility is ever acceptable, we're going to seriously
limit our opportunities to revisit poor design decisions.

I think there's a funny kind of thing that happens when we discuss a
behavior that is sub-optimal: we start to look for ways to justify
leaving it the way it is, because surely it couldn't be a terrible
idea if it's been like that forever.  I think there's some of that
going on on the thread about stripping trailing null columns, too: if
we've got a benchmark result showing that the patch saves CPU time on
a 5-column table (!), then all the pontificating about 700-column
tables being rare is irrelevant.  Similarly here: it's true that
someone might have to revisit their init scripts, but should they fail
to do so, the consequences are really not that dire.

On the other hand, in PostgreSQL 8.4, we changed TRUNCATE to wipe out
the entire inheritance hierarchy instead of only the named table
(unless the new ONLY keyword was added).  This obviously has the
potential to be completely disastrous for someone with a very
particular usage pattern, but there was little discussion and everyone
basically said "yeah, we should go ahead and change that, despite the
small risk that someone will accidentally blow away a lot more data
than they intended".  Maybe there are more people using smart shutdown
than there are people truncating only the root of an inheritance
hierarchy, but nothing we're proposing to do here is going to
permanently erase anyone's data, either.

I don't think you can use the TRUNCATE case as an example. For me,
that was a prime case of insufficient discussion around the principle
of backwards compatibility. It wasn't clear to me that was happening
and had I known, I would have objected. IIRC the first I knew of it
was when the release notes came out months after things were settled.

We go to great lengths to note initdb inducing behaviour during beta,
but very little towards behaviour changes that require downstream
software changes.

Maybe we don't need to do this over multiple releases, but we do need
to give warning of possible incompatibilities. It would be good to see
a specific post on hackers called "Planned Incompatibilities in 9.2",
or collect such things on the open items wiki, so that people
listening can see what might happen and get a chance to object. Or if
changes do go ahead, at least we give them a few months warning to
change the downstream software. Otherwise all that happens is our new
release comes out and fewer people use it because it takes ages to
actually realign the software stack enough for our software to be
used.

The better we succeed at persuading the world to use Postgres the more
important backwards compatibility becomes. When fewer people used
Postgres it was easy to charge forwards aggressively, but as we begin
to lead we must be more careful.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#29Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#26)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Peter Eisentraut <peter_e@gmx.net> writes:

In any case, if either the existing session of the TM is cut or it
cannot create a new connection, it will, after some time, have to give
up roll back the prepared transactions on the other servers. So some
kind of setting to not shut down if there are prepared transactions
pending could be useful. But this could probably be a separate GUC
setting or two instead of a shutdown mode (or two) of its own.

This argument still seems pretty bogus. The *entire* point of a TM
is to cope with crashes of individual databases under its management.
The proposed setting seems to amount to a "please don't crash" GUC,
which is silly on its face, and does not actually make the TM's life
any easier anyway.

regards, tom lane

#30Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#27)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Peter Eisentraut <peter_e@gmx.net> writes:

On fre, 2012-04-27 at 14:57 -0400, Robert Haas wrote:

I think there is no point at all in having a discussion about this
unless we can first agree that the overwhelming majority of people who
have commented on this issue on this list are unhappy with the current
default behavior. If we are not going to change the default behavior,
then there is zero point in talking about this.

Have you reviewed the previous discussions where changing the default
behavior was discussed and rejected? I don't like the current default
any more than you do, but without any new arguments, there is, as you
say, zero point in talking about this.

Perhaps I've forgotten something, but I only recall debates about
switching the default to a different one of the existing shutdown modes.
The new material here is the proposal for a new mode.

regards, tom lane

#31Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#29)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Sun, Apr 29, 2012 at 4:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

In any case, if either the existing session of the TM is cut or it
cannot create a new connection, it will, after some time, have to give
up roll back the prepared transactions on the other servers.  So some
kind of setting to not shut down if there are prepared transactions
pending could be useful.  But this could probably be a separate GUC
setting or two instead of a shutdown mode (or two) of its own.

This argument still seems pretty bogus.  The *entire* point of a TM
is to cope with crashes of individual databases under its management.
The proposed setting seems to amount to a "please don't crash" GUC,
which is silly on its face, and does not actually make the TM's life
any easier anyway.

You are right that the TM can cope with aborted transactions, but that
doesn't mean we should force it to have to do that. If we can wait for
commit then we should do so.

I think we only need one new mode, "shutdown when transactions are
finished" should only shutdown when all types of transaction are
complete. For people that don't use prepared transactions the
difference is irrelevant. For people that do use prepared
transactions, I can't imagine they would want a new setting that ends
with aborted transactions, since that isn't any different to a fast
shutdown.

If that hangs waiting for TM that has gone away, then you can issue
shutdown fast.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#32Tom Lane
tgl@sss.pgh.pa.us
In reply to: Simon Riggs (#31)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Simon Riggs <simon@2ndQuadrant.com> writes:

I think we only need one new mode, "shutdown when transactions are
finished" should only shutdown when all types of transaction are
complete. For people that don't use prepared transactions the
difference is irrelevant. For people that do use prepared
transactions, I can't imagine they would want a new setting that ends
with aborted transactions, since that isn't any different to a fast
shutdown.

That sounds reasonable at first blush. Implementing it might be
trickier than you think though, since (despite Peter's opinion) the
prepared xacts are not associated with any particular session, and the
postmaster itself doesn't know they are there. What's more, if
individual sessions are told to commit hara-kiri as soon as they are not
in a transaction, there soon won't be any surviving session in which the
TM could issue a COMMIT PREPARED.

I think the only way this could be made to fly would be if the TM could
set a session state that indicates "I'm a TM session, don't kill me
until all prepared transactions are gone". Which might be problematic
from a security standpoint, if random users could use it to proof
themselves against getting kicked out. We could make it SUSET but then
TMs would have to run as superuser, which seems a bit less than
desirable.

On the whole it is not apparent to me that we really need a mode in
which shutdown waits for prepared transactions to flush out; and I would
definitely not be in favor of it being the default. I think that that
would make prepared transactions an even bigger foot-gun than they are
now. Just think: you say "pg_ctl stop", and the server promptly kicks
off all your users and won't let any more in, but doesn't actually shut
down. You may not know why, and even if you do, you can't connect to do
something about it. Eventually you give up and issue shutdown fast,
cursing whoever designed that misbegotten behavior.

regards, tom lane

#33Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#32)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Sun, Apr 29, 2012 at 5:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Simon Riggs <simon@2ndQuadrant.com> writes:

I think we only need one new mode, "shutdown when transactions are
finished" should only shutdown when all types of transaction are
complete. For people that don't use prepared transactions the
difference is irrelevant. For people that do use prepared
transactions, I can't imagine they would want a new setting that ends
with aborted transactions, since that isn't any different to a fast
shutdown.

That sounds reasonable at first blush.  Implementing it might be
trickier than you think though, since (despite Peter's opinion) the
prepared xacts are not associated with any particular session, and the
postmaster itself doesn't know they are there.  What's more, if
individual sessions are told to commit hara-kiri as soon as they are not
in a transaction, there soon won't be any surviving session in which the
TM could issue a COMMIT PREPARED.

I think the only way this could be made to fly would be if the TM could
set a session state that indicates "I'm a TM session, don't kill me
until all prepared transactions are gone".  Which might be problematic
from a security standpoint, if random users could use it to proof
themselves against getting kicked out.  We could make it SUSET but then
TMs would have to run as superuser, which seems a bit less than
desirable.

I think an explicit state is overkill and has other problems as you say.

On the whole it is not apparent to me that we really need a mode in
which shutdown waits for prepared transactions to flush out; and I would
definitely not be in favor of it being the default.  I think that that
would make prepared transactions an even bigger foot-gun than they are
now.  Just think: you say "pg_ctl stop", and the server promptly kicks
off all your users and won't let any more in, but doesn't actually shut
down.  You may not know why, and even if you do, you can't connect to do
something about it.  Eventually you give up and issue shutdown fast,
cursing whoever designed that misbegotten behavior.

Waiting too long is clearly a foot fun, as you say.

But if you just issued PREPARE on a session, its more than likely that
this will be followed almost immediately by a COMMIT. Simply waiting
is a good indication, and some reasonable time like 10 seconds is fine
in determining whether that COMMIT will arrive, or not.

This only matters on a shutdown. If its a restart, we can shutdown
after a PREPARE because as soon as we are back up again the TM can
issue the COMMIT.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#34Peter Eisentraut
peter_e@gmx.net
In reply to: Simon Riggs (#28)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On sön, 2012-04-29 at 10:19 +0100, Simon Riggs wrote:

Maybe we don't need to do this over multiple releases, but we do need
to give warning of possible incompatibilities. It would be good to see
a specific post on hackers called "Planned Incompatibilities in 9.2",
or collect such things on the open items wiki, so that people
listening can see what might happen and get a chance to object. Or if
changes do go ahead, at least we give them a few months warning to
change the downstream software. Otherwise all that happens is our new
release comes out and fewer people use it because it takes ages to
actually realign the software stack enough for our software to be
used.

Well, either there are possible incompatibilities, in which case users
will be slow to adopt new releases, as is currently the case, or there
strictly won't be any (unless hidden behind config settings or similar),
but then introducing new features or bug fixes can take many years. So
far we've erred on the side of "progress".

#35Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#34)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Sun, Apr 29, 2012 at 1:48 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On sön, 2012-04-29 at 10:19 +0100, Simon Riggs wrote:

Maybe we don't need to do this over multiple releases, but we do need
to give warning of possible incompatibilities. It would be good to see
a specific post on hackers called "Planned Incompatibilities in 9.2",
or collect such things on the open items wiki, so that people
listening can see what might happen and get a chance to object. Or if
changes do go ahead, at least we give them a few months warning to
change the downstream software. Otherwise all that happens is our new
release comes out and fewer people use it because it takes ages to
actually realign the software stack enough for our software to be
used.

Well, either there are possible incompatibilities, in which case users
will be slow to adopt new releases, as is currently the case, or there
strictly won't be any (unless hidden behind config settings or similar),
but then introducing new features or bug fixes can take many years.  So
far we've erred on the side of "progress".

"Erred on the side of progress" might even be a little strong, because
I think for the most part we have been extremely judicious about
backward incompatibilities in the last few releases (which is a good
thing). Obviously, 8.3 was a flag day of the first magnitude, and one
I hope we won't repeat any time soon, but if you look through the
release notes for, say, 9.1, just about every "incompatibility" listed
there amounts to fixing something that was either demonstrably broken
or widely hated in prior releases. Turning on
standard_conforming_strings by default was a big deal, but we've been
phasing that change in for five years or so, so I think we really did
about as much to ease that transition as is humanly possible.
Moreover, you can always turn the GUC off again, if the new behaviour
is a problem.

The only way we're going to have fewer incompatibilities than we do
now is to preserve existing behavior even when it's broken,
widely-hated, and/or not standards-conformant. IMHO, that would be
taking a sound principle to an illogical extreme.

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

#36Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#35)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Robert Haas <robertmhaas@gmail.com> writes:

"Erred on the side of progress" might even be a little strong, because
I think for the most part we have been extremely judicious about
backward incompatibilities in the last few releases (which is a good
thing). Obviously, 8.3 was a flag day of the first magnitude, and one
I hope we won't repeat any time soon, but if you look through the
release notes for, say, 9.1, just about every "incompatibility" listed
there amounts to fixing something that was either demonstrably broken
or widely hated in prior releases.

Well, if you're ragging on the implicit coercions changes, let me point
out that those were also fixing something that was demonstrably broken.
So I'm afraid it's a tad pollyanna-ish to claim that there is never
going to be push-back on changes we choose to make for one or another
of these reasons.

Having said that, though, I agree that we have to be willing to make
incompatible changes from time to time, and I think our standards for
when to do that are plenty high enough already. I'm not in favor of
raising that bar still more. The reason we support back branches as
long as we do is precisely to give people the option to not deal with
incompatible changes until they are ready to. I don't think we need
to do even more, and I don't want to add still more overhead to the
development process to do it.

regards, tom lane

#37Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#36)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Sun, Apr 29, 2012 at 5:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

"Erred on the side of progress" might even be a little strong, because
I think for the most part we have been extremely judicious about
backward incompatibilities in the last few releases (which is a good
thing).  Obviously, 8.3 was a flag day of the first magnitude, and one
I hope we won't repeat any time soon, but if you look through the
release notes for, say, 9.1, just about every "incompatibility" listed
there amounts to fixing something that was either demonstrably broken
or widely hated in prior releases.

Well, if you're ragging on the implicit coercions changes, let me point
out that those were also fixing something that was demonstrably broken.

True, but it was painful for a lot of people, and I continue to think
that we broke too many reasonable cases.

So I'm afraid it's a tad pollyanna-ish to claim that there is never
going to be push-back on changes we choose to make for one or another
of these reasons.

Agreed, I expect some push-back. I'm just pointing out that we reject
a very significant number of changes on backward-compatibility
grounds. We don't reject too many entire patches on these grounds,
but many are the patch authors who have been asked to change X,Y, or Z
to avoid breaking backward compatibility, or who have had things
ripped out by the committer for such reasons. Of course this is
sometimes an occasion for complaint, and then the backward
compatibility changes that do get through are also an occasion for
complaint, so there's no perfect world, but we do try pretty hard, I
think.

Having said that, though, I agree that we have to be willing to make
incompatible changes from time to time, and I think our standards for
when to do that are plenty high enough already.  I'm not in favor of
raising that bar still more.  The reason we support back branches as
long as we do is precisely to give people the option to not deal with
incompatible changes until they are ready to.  I don't think we need
to do even more, and I don't want to add still more overhead to the
development process to do it.

+1, and well put.

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

#38Albe Laurenz
laurenz.albe@wien.gv.at
In reply to: Tom Lane (#8)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Tom Lane wrote:

On Fri, Apr 27, 2012 at 7:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

No, I'm not happy with that. Smart shutdown is defined to not

affect

current sessions. I'm fine with having a fourth mode that acts as

you

suggest (and, probably, even with making it the default); but not

with

taking away a behavior that people may well be relying on.

Agreed, but not sure what to call the new mode: "smarter"?

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old

"smart"

behavior. How about

slow - allow existing sessions to finish (old "smart")
smart - allow existing transactions to finish (new)
fast - kill active queries
immediate - unclean shutdown

But if the meaning of "smart" changes, then people who use
"pg_ctl stop -m smart" and expect that active sessions will not be
affected will get a surprise.

Wouldn't it be better to pick a different name for the new fourth
mode? It could still be the default mode, but I think that people
who explicitly specify a certain mode are more likely to care about
the exact behaviour.

I second Heikki's suggestions for mode names.

And +1 from me on changing the default behaviour.

Yours,
Laurenz Albe

#39Wolfgang Wilhelm
wolfgang20121964@yahoo.de
In reply to: Tom Lane (#8)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Just for the ones interested in a view on another turf:

In Oracle "shutdown immediate" is the fastest _clean_ shutdown and "shutdown abort" is equal to "shutdown immediate" in PG.
The other modes are called "shutdown normal" and "shutdown transactional".

Wolfgang

________________________________
Von: Tom Lane <tgl@sss.pgh.pa.us>
An: Simon Riggs <simon@2ndQuadrant.com>
CC: Robert Haas <robertmhaas@gmail.com>; Alvaro Herrera <alvherre@commandprompt.com>; Magnus Hagander <magnus@hagander.net>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Gesendet: 20:48 Freitag, 27.April 2012
Betreff: Re: [HACKERS] smart shutdown at end of transaction (was: Default mode for shutdown)

Simon Riggs <simon@2ndQuadrant.com> writes:

On Fri, Apr 27, 2012 at 7:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

No, I'm not happy with that.  Smart shutdown is defined to not affect
current sessions.  I'm fine with having a fourth mode that acts as you
suggest (and, probably, even with making it the default); but not with
taking away a behavior that people may well be relying on.

Agreed, but not sure what to call the new mode: "smarter"?

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior.  How about

    slow    - allow existing sessions to finish (old "smart")
    smart    - allow existing transactions to finish (new)
    fast    - kill active queries
    immediate - unclean shutdown

            regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#40Greg Stark
stark@mit.edu
In reply to: Wolfgang Wilhelm (#39)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Mon, Apr 30, 2012 at 9:55 AM, Wolfgang Wilhelm
<wolfgang20121964@yahoo.de> wrote:

Just for the ones interested in a view on another turf:

In Oracle "shutdown immediate" is the fastest _clean_ shutdown and "shutdown
abort" is equal to "shutdown immediate" in PG.
The other modes are called "shutdown normal" and "shutdown transactional".

Though the behaviour users see is quite different. In Oracle the
fastest clean shutdown still requires rolling back transactions which
can take a long time. In Postgres rolling back transactions is
instantaneous so a shutdown immediate will appear to behave like a
shutdown abort in Oracle in that it will always run fast even if the
effect on the database is different.

--
greg

#41Bruce Momjian
bruce@momjian.us
In reply to: Simon Riggs (#28)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Sun, Apr 29, 2012 at 10:19:38AM +0100, Simon Riggs wrote:

Maybe we don't need to do this over multiple releases, but we do need
to give warning of possible incompatibilities. It would be good to see
a specific post on hackers called "Planned Incompatibilities in 9.2",
or collect such things on the open items wiki, so that people
listening can see what might happen and get a chance to object. Or if
changes do go ahead, at least we give them a few months warning to
change the downstream software. Otherwise all that happens is our new
release comes out and fewer people use it because it takes ages to
actually realign the software stack enough for our software to be
used.

The release notes would certainly feature this as an incompatibility,
and would be present even before beta started. Unless they skip reading
the release notes, it would be hard for them to miss this change. I
also blog when major release notes are available for viewing.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

#42Fujii Masao
masao.fujii@gmail.com
In reply to: Robert Haas (#12)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Sat, Apr 28, 2012 at 4:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Apr 27, 2012 at 2:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior.  How about

       slow    - allow existing sessions to finish (old "smart")
       smart   - allow existing transactions to finish (new)
       fast    - kill active queries
       immediate - unclean shutdown

I could live with that.  Really, I'd like to have fast just be the
default.  But the above compromise would still be a big improvement
over what we have now, assuming the new smart becomes the default.

Should this new shutdown mode wait for online backup like old "smart" does?

Regards,

--
Fujii Masao

#43Albe Laurenz
laurenz.albe@wien.gv.at
In reply to: Fujii Masao (#42)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

Fujii Masao wrote:

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior.  How about

       slow    - allow existing sessions to finish (old "smart")
       smart   - allow existing transactions to finish (new)
       fast    - kill active queries
       immediate - unclean shutdown

I could live with that.  Really, I'd like to have fast just be the
default.  But the above compromise would still be a big improvement
over what we have now, assuming the new smart becomes the default.

Should this new shutdown mode wait for online backup like old "smart" does?

I think it shouldn't; I like to think of it as some kind of "quite fast"
shutdown (provided there are no long-running transactions).

And I still think that we should choose a name different from "smart"
to indicate that something has changed, even if it is the new default.

Yours,
Laurenz Albe

#44Robert Haas
robertmhaas@gmail.com
In reply to: Fujii Masao (#42)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Sat, May 5, 2012 at 12:41 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Sat, Apr 28, 2012 at 4:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Apr 27, 2012 at 2:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior.  How about

       slow    - allow existing sessions to finish (old "smart")
       smart   - allow existing transactions to finish (new)
       fast    - kill active queries
       immediate - unclean shutdown

I could live with that.  Really, I'd like to have fast just be the
default.  But the above compromise would still be a big improvement
over what we have now, assuming the new smart becomes the default.

Should this new shutdown mode wait for online backup like old "smart" does?

I think it had better not, because what happens when all the
connections are gone, no new ones can be made, and yet online backup
mode is still active?

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

#45Fujii Masao
masao.fujii@gmail.com
In reply to: Robert Haas (#44)
Re: smart shutdown at end of transaction (was: Default mode for shutdown)

On Tue, May 8, 2012 at 12:59 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Sat, May 5, 2012 at 12:41 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Sat, Apr 28, 2012 at 4:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Apr 27, 2012 at 2:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm not necessarily opposed to commandeering the name "smart" for the
new behavior, so that what we have to find a name for is the old "smart"
behavior.  How about

       slow    - allow existing sessions to finish (old "smart")
       smart   - allow existing transactions to finish (new)
       fast    - kill active queries
       immediate - unclean shutdown

I could live with that.  Really, I'd like to have fast just be the
default.  But the above compromise would still be a big improvement
over what we have now, assuming the new smart becomes the default.

Should this new shutdown mode wait for online backup like old "smart" does?

I think it had better not, because what happens when all the
connections are gone, no new ones can be made, and yet online backup
mode is still active?

Yep, I agree that new mode should not. This change of the default shutdown
behavior might surprise some users, so it's better to document also this in
release note.

Regards,

--
Fujii Masao