Thread: Re: default_transaction_isolation = serializable causes crash under Hot Standby

> "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

On Sat, Apr 28, 2012 at 5:56 AM, Kevin Grittner
<Kevin.Grittner@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

On Sun, Apr 29, 2012 at 8:20 AM, Simon Riggs <simon@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

Robert Haas <robertmhaas@gmail.com> wrote:
> Simon Riggs <simon@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

On Mon, Apr 30, 2012 at 10:26 AM, Kevin Grittner
<Kevin.Grittner@wicourts.gov> wrote:
> Robert Haas <robertmhaas@gmail.com> wrote:
>> Simon Riggs <simon@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