default_transaction_isolation = serializable causes crash under Hot Standby

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

Robert Haas <robertmhaas(at)gmail(dot)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

On Fri, Apr 27, 2012 at 10:21 AM, Kevin Grittner
<Kevin(dot)Grittner(at)wicourts(dot)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

Robert Haas <robertmhaas(at)gmail(dot)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

On Fri, Apr 27, 2012 at 11:02 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)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

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)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

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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