Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-30 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 Simon Riggs si...@2ndquadrant.com wrote:
 
 * throw a WARNING if serializable is stated in other cases, and
 downgrade the request to repeatable read
 
 I think this would be reasonable, but it's still my second choice.
 The advantage of throwing an ERROR is that someone will presumably
 be forced to realize that a problem exists and fix it, whereas a
 WARNING may just generate a combination of log spam and unexpected
 behavior forever.  Also, we currently block cases where you try to
 set transaction_isolation by throwing an ERROR, so it seems a bit
 more consistent to do that in other cases as well.  Still, it's a
 reasonable choice, and certainly better than failing an assertion.
 
I'm not totally clear on your first choice.  Are you looking for
something similar to the patch I posted, except that it would dodge
all resulting errors at the point where they are promoted to FATAL
(before HS is really functional)?
 
-Kevin

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-30 Thread Robert Haas
On Mon, Apr 30, 2012 at 10:26 AM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Robert Haas robertmh...@gmail.com wrote:
 Simon Riggs si...@2ndquadrant.com wrote:

 * throw a WARNING if serializable is stated in other cases, and
 downgrade the request to repeatable read

 I think this would be reasonable, but it's still my second choice.
 The advantage of throwing an ERROR is that someone will presumably
 be forced to realize that a problem exists and fix it, whereas a
 WARNING may just generate a combination of log spam and unexpected
 behavior forever.  Also, we currently block cases where you try to
 set transaction_isolation by throwing an ERROR, so it seems a bit
 more consistent to do that in other cases as well.  Still, it's a
 reasonable choice, and certainly better than failing an assertion.

 I'm not totally clear on your first choice.  Are you looking for
 something similar to the patch I posted, except that it would dodge
 all resulting errors at the point where they are promoted to FATAL
 (before HS is really functional)?

That's my vote.  Where is that FATAL error coming from?  I'm guessing
it's somehow resulting from failure to set up the startup transaction
in InitPostgres().  If that's the case, we ought to be able to work
around it, because surely repeatable read would be fine for the
startup transaction, which doesn't really do anything anyway.

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

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-29 Thread Simon Riggs
On Sat, Apr 28, 2012 at 5:56 AM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:

 But if you set it in the postgresql.conf file, it's not pretty:

 kevin@kevin-desktop:~$ psql -p 5433 test
 psql: FATAL:  can not create a serializable snapshot during recovery

 Ideas?

The patch as submitted doesn't do anything useful for the case where
the setting in .conf is serializable. It just throws an ERROR for any
action, which is not useful or friendly.

IMHO the desired behaviour would be

* prevent default_transaction_isolation = serializable as a default
setting when we enter Hot Standby by throwing a FATAL error from the
startup process. I can help implement that if we agree.

* throw a WARNING if serializable is stated in other cases, and
downgrade the request to repeatable read

Throwing a WARNING is better than an ERROR, since it reduces the level
of application editing to make it work for HS. Or better still, just
document that serializable currently means repeatable read when
executed on an HS.

On another note, what happened to the plan to export regular
serializable snapshots to the standby for use as deferred snapshots?

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

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-29 Thread Kevin Grittner
Simon Riggs  wrote:
 Kevin Grittner  wrote:
 
 But if you set it in the postgresql.conf file, it's not pretty:

 kevin@kevin-desktop:~$ psql -p 5433 test
 psql: FATAL: can not create a serializable snapshot during
 recovery

 Ideas?
 
 The patch as submitted doesn't do anything useful for the case
 where the setting in .conf is serializable.
 
Precisely the point I was trying to make.  We discussed an approach,
I tried it, but that approach doesn't work.  So we need new ideas.
 
 IMHO the desired behaviour would be
 
 * prevent default_transaction_isolation = serializable as a default
 setting when we enter Hot Standby by throwing a FATAL error from
 the startup process. I can help implement that if we agree.
 
I'm not sure how well that plays for all users of HS.  It would work
for us because there are a number of settings we want to change on a
HS; we use a different postgresql.conf from the master, so this works
fine.  I am concerned about people who want to promote the HS to a
new master without swapping in a different conf file and doing a
reload or restart -- if they want serializable transactions as the
default on their master, how would this work?
 
 * throw a WARNING if serializable is stated in other cases, and
 downgrade the request to repeatable read
 
Yeah, that idea has been in the back of my mind all along, but I
couldn't find a comfortable way to reconcile that with the point
above and with the fact that we all seem to hope to some day have a
way to run true serializable transactions under HS.
 
 Throwing a WARNING is better than an ERROR, since it reduces the
 level of application editing to make it work for HS. Or better
 still, just document that serializable currently means repeatable
 read when executed on an HS.
 
Somehow it just feels wrong, but if nobody can propose a less
painful alternative, perhaps that is best.  For now.
 
 On another note, what happened to the plan to export regular
 serializable snapshots to the standby for use as deferred
 snapshots?
 
For the record, I think the most promising variation on that was to
include a new WAL record type or a bit in the WAL record header to
flag points in the WAL stream at which a transaction on HS could not
see any anomalies.  A serializable transaction on a HS would use a
snapshot built at that point, either by waiting for one or having the
WAL receiver keep the latest known safe one available.
 
That is still on my list of things I would like to do, but round
tuits are hard to come by these days, and there are a number of items
higher on that list.
 
-Kevin

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-29 Thread Simon Riggs
On Sun, Apr 29, 2012 at 1:40 PM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:

 IMHO the desired behaviour would be

 * prevent default_transaction_isolation = serializable as a default
 setting when we enter Hot Standby by throwing a FATAL error from
 the startup process. I can help implement that if we agree.

 I'm not sure how well that plays for all users of HS.  It would work
 for us because there are a number of settings we want to change on a
 HS; we use a different postgresql.conf from the master, so this works
 fine.  I am concerned about people who want to promote the HS to a
 new master without swapping in a different conf file and doing a
 reload or restart -- if they want serializable transactions as the
 default on their master, how would this work?

I can see you might want to have a standby with hot_standby=off that
was immediately ready to take the place of the master. In that case,
the setting of default_transaction_isolation would have no effect on
the standby, so you are ready and waiting.

If you request hot_standby=on presumably you're not going to want a
continuous stream of ERRORs. Blocking that completely is hard because
we would need to test the parameter file as well as testing user or
database settings.

The only way default_transaction_isolation = serializable would be
acceptable when hot_standby = on is if we silently downgrade the
isolation level to read committed. That way everything just works,
albeit not quite as requested. So I think that's the best way
forwards.

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

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-29 Thread Kevin Grittner
Simon Riggs  wrote:
 
 The only way default_transaction_isolation = serializable would be
 acceptable when hot_standby = on is if we silently downgrade the
 isolation level to read committed. That way everything just works,
 albeit not quite as requested. So I think that's the best way
 forwards.
 
I guess I don't see much alternative.  Did you want to code that (I
seem to remember you saying that in an earlier post) or would you
like me to do it?
 
-Kevin

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-29 Thread Simon Riggs
On Sun, Apr 29, 2012 at 5:54 PM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Simon Riggs  wrote:

 The only way default_transaction_isolation = serializable would be
 acceptable when hot_standby = on is if we silently downgrade the
 isolation level to read committed. That way everything just works,
 albeit not quite as requested. So I think that's the best way
 forwards.

 I guess I don't see much alternative.  Did you want to code that (I
 seem to remember you saying that in an earlier post) or would you
 like me to do it?

I'll happily do that, unless you wish to.

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

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-29 Thread Kevin Grittner
 Simon Riggs  wrote:
 On Sun, Apr 29, 2012 at 5:54 PM, Kevin Grittner
  wrote:
 Simon Riggs wrote:

 The only way default_transaction_isolation = serializable would
 be acceptable when hot_standby = on is if we silently downgrade
 the isolation level to read committed. That way everything just
 works, albeit not quite as requested. So I think that's the best
 way forwards.

 I guess I don't see much alternative. Did you want to code that (I
 seem to remember you saying that in an earlier post) or would you
 like me to do it?
 
 I'll happily do that, unless you wish to.
 
I think I know where to do that.  I'll give it a shot.
 
-Kevin

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-29 Thread Robert Haas
On Sun, Apr 29, 2012 at 8:20 AM, Simon Riggs si...@2ndquadrant.com wrote:
 * prevent default_transaction_isolation = serializable as a default
 setting when we enter Hot Standby by throwing a FATAL error from the
 startup process. I can help implement that if we agree.

I am strongly disinclined to go that route, because (1) our customers,
at least, really hate it when the standby won't start up, and I think
we should be very careful about finding more reasons for that to
happen; and (2) it's not bullet-proof anyway, because there is still
ALTER USER .. SET and ALTER DATABASE .. SET and maybe a few other
methods as well.

Keep in mind, also, that this has to be back-patched to 9.1; so if we
go this route then someone might shut down their server, upgrade the
binaries, restart the server, and have it fail to start.  I think
that's not a nice thing to do in a minor release.

 * throw a WARNING if serializable is stated in other cases, and
 downgrade the request to repeatable read

I think this would be reasonable, but it's still my second choice.
The advantage of throwing an ERROR is that someone will presumably be
forced to realize that a problem exists and fix it, whereas a WARNING
may just generate a combination of log spam and unexpected behavior
forever.  Also, we currently block cases where you try to set
transaction_isolation by throwing an ERROR, so it seems a bit more
consistent to do that in other cases as well.  Still, it's a
reasonable choice, and certainly better than failing an assertion.

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

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


[HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-27 Thread Robert Haas
When I do this:

rhaas=# set default_transaction_isolation = 'serializable';
SET
rhaas=# begin;
BEGIN
rhaas=# select 1;

Then I get this:

TRAP: FailedAssertion(!(!RecoveryInProgress()), File: predicate.c,
Line: 1637)
LOG:  server process (PID 290) was terminated by signal 6: Abort trap

The root of the problem here seems to be that we're imagining that
it's possible to prevent serializable mode from being used under HS
from within the check function for the transaction_isolation GUC - see
check_XactIsoLevel.  However, because there's a second GUC
(default_transaction_isolation) that can also be used to change the
initial setting, it doesn't work.

I'm not exactly sure what the best way to fix this is.  I assume that
this problem also exists in 9.1, but I haven't checked.

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

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-27 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 
 When I do this:
 
 rhaas=# set default_transaction_isolation = 'serializable';
 SET
 rhaas=# begin;
 BEGIN
 rhaas=# select 1;
 
 Then I get this:
 
 TRAP: FailedAssertion(!(!RecoveryInProgress()), File:
 predicate.c, Line: 1637)
 LOG:  server process (PID 290) was terminated by signal 6: Abort
 trap
 
 The root of the problem here seems to be that we're imagining that
 it's possible to prevent serializable mode from being used under
 HS from within the check function for the transaction_isolation
 GUC - see check_XactIsoLevel.  However, because there's a second
 GUC (default_transaction_isolation) that can also be used to
 change the initial setting, it doesn't work.
 
 I'm not exactly sure what the best way to fix this is.  I assume
 that this problem also exists in 9.1, but I haven't checked.
 
My first thought was that if we can detect that we are in HS, we
should probably throw an ERROR on an attempt to set
default_transaction_isolation = 'serializable'.  But that raises the
question about what to do with environments where people want the
master to be running with that default (set from postgresql.conf) --
fail-over could be, well, interesting.  We haven't run into this
situation yet because we don't use our production postgresql.conf on
our hot standbys -- when we make a base backup we rename
postgresql.conf to postgresql.conf.production and copy in a special
configuration file.  For those who want the same configuration on
both, and who want to use serializable transactions on the master, I
don't see a really clean solution.  Does anyone else?

We didn't want to allow SERIALIZABLE to be set while still allowing
read-only anomalies like this (assuming T3 was on the HS):
 
http://wiki.postgresql.org/wiki/SSI#Deposit_Report
 
-Kevin

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-27 Thread Robert Haas
On Fri, Apr 27, 2012 at 10:21 AM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 My first thought was that if we can detect that we are in HS, we
 should probably throw an ERROR on an attempt to set
 default_transaction_isolation = 'serializable'.

I think that would result in the server failing to start.  We could
throw a warning there and give repeatable read.

Or, maybe there's a way to throw an error when serializable mode is
used rather than when it's requested.  So in the above example SELECT
1; would say, hey, somehow I ended up in serializable mode under HS,
abort, abort!

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

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-27 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Or, maybe there's a way to throw an error when serializable mode is
 used rather than when it's requested.

Couldn't we check and throw an error at the place in transaction startup
where default_transaction_isolation is copied to the active variable?

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-27 Thread Robert Haas
On Fri, Apr 27, 2012 at 11:02 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Or, maybe there's a way to throw an error when serializable mode is
 used rather than when it's requested.

 Couldn't we check and throw an error at the place in transaction startup
 where default_transaction_isolation is copied to the active variable?

Yeah, possibly.  Although the user might still use SET TRANSACTION
ISOLATION, so it would ideally be nice to postpone throwing the error
until after that opportunity is past.  Not sure if that's feasible,
though.

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

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-27 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Or, maybe there's a way to throw an error when serializable mode
 is used rather than when it's requested.
 
 Couldn't we check and throw an error at the place in transaction
 startup where default_transaction_isolation is copied to the
 active variable?
 
Wouldn't that leave users stuck if the postgresql.conf set the
default to serializable?  Nobody would be able to start a
transaction, even to change the default, would they?  If that's the
case, we might as well refuse to start.
 
Robert's suggestion might be the least of the various evils.
 
-Kevin

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-27 Thread Tom Lane
Kevin Grittner kevin.gritt...@wicourts.gov writes:
 Tom Lane t...@sss.pgh.pa.us wrote:
 Couldn't we check and throw an error at the place in transaction
 startup where default_transaction_isolation is copied to the
 active variable?
 
 Wouldn't that leave users stuck if the postgresql.conf set the
 default to serializable?  Nobody would be able to start a
 transaction, even to change the default, would they?

I was assuming BEGIN TRANSACTION LEVEL ... would still work;
if not, it's a non-starter.  I haven't looked at the code to see
if the sequence of operations is amenable to that though.

 Robert's suggestion might be the least of the various evils.

Yeah, it would definitely be nicer if BEGIN; SET TRANSACTION LEVEL
would work too.  Maybe the place to put the check is where we
establish the transaction snapshot.

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-27 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 
 Yeah, it would definitely be nicer if BEGIN; SET TRANSACTION LEVEL
 would work too.  Maybe the place to put the check is where we
 establish the transaction snapshot.
 
That makes sense, and doesn't seem like it would be hard, from what
I recall of that code.  I know I've fallen down on a couple other
things which I meant to look at lately, but this one should be small
enough to fit in between the brickbats the non-PostgreSQL world has
been throwing at me lately.  I'll take a shot at it sometime today,
although it may be late.
 
-Kevin

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


Re: [HACKERS] default_transaction_isolation = serializable causes crash under Hot Standby

2012-04-27 Thread Kevin Grittner
 Kevin Grittner  wrote:
 Tom Lane  wrote:
 
 Yeah, it would definitely be nicer if BEGIN; SET TRANSACTION LEVEL
 would work too. Maybe the place to put the check is where we
 establish the transaction snapshot.
 
 That makes sense, 
 
 I'll take a shot at it sometime today,
 
Attached.  With a default setting in postgresql.conf it handles
Robert's test case nicely, and does what Tom asks for above:

test=# set default_transaction_isolation = 'serializable';
SET
test=# begin;
BEGIN
test=# select 1;
ERROR:  can not create a serializable snapshot during recovery
test=# rollback;
ROLLBACK
test=# begin;
BEGIN
test=# set transaction isolation level repeatable read;
SET
test=# select 1;
 ?column? 
--
1
(1 row)

test=# commit;
COMMIT

But if you set it in the postgresql.conf file, it's not pretty:
 
kevin@kevin-desktop:~$ psql -p 5433 test
psql: FATAL:  can not create a serializable snapshot during recovery
 
Ideas?
 
-Kevin




hs-default-serializable-v1.diff
Description: Binary data

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