Re: default_isolation_level='serializable' crashes on Windows

On 14.08.2012 14:25, Kevin Grittner wrote:
> Heikki Linnakangas wrote:
>> On 14.08.2012 08:23, Kevin Grittner wrote:
>
>>> OK, attached is a first cut to confirm that the approach looks
>>> sane to everyone; I *think* it is along the lines that we reached
>>> consensus. After moving the check to the point where we get a
>>> serializable snapshot, it was still behaving badly. It took a bit,
>>> but forcing the snapshot acquisition in InitPostgres to use 'read
>>> committed' instead of the default isolation level got reasonable
>>> behavior in my initial tests. It sure looks a lot better to *me*
>>> than what was happening before.
>
> Oh, further testing this morning shows that while *queries* on the HS
> seem OK, streaming replication is now broken. I probably need to
> override transaction isolation on the recovery process. I'll take a
> look at that.

Hmm, seems to work for me. Do you get an unexpected error or what?

>> Now that the error is thrown when the first snapshot is taken,
>> rather than at the SET command, I think the error message needs
>> some work:
>>
>> postgres=# select 123;
>> ERROR: cannot use serializable mode in a hot standby
>> HINT: You can use REPEATABLE READ instead.
>>
>> If the isolation level came from default_transaction_isolation, set
>> in postgresql.conf file, it might take the user a while to figure
>> that out. Perhaps something like this:
>>
>> ERROR: cannot use serializable mode in a hot standby
>> DETAIL: default_transaction_isolation was set to 'serializable' in
>> the config file.
>> HINT: You can use "SET transaction_isolation = 'repeatable read'"
>> before the first query in the transaction to override it.
>
> Did you mean?:
>
> HINT: You can use "SET default_transaction_isolation = 'repeatable
> read'" before the first query in the transaction to override it.
>
> Otherwise, I agree and will do.

I didn't, I meant to point out that you can set transaction_isolation
just for the one transaction. But your suggested hint is OK as well -
you suggest to set it for the whole session, which will also work around
the problem. The "before the first query in the transaction" isn't
necessary in that case though.

Yet another option is to suggest the "BEGIN ISOLATION LEVEL REPEATABLE
READ" syntax, instead of SET.

>>> Since the existing behavior is so bad, I'm inclined to think this
>>> merits backpatching to 9.1. Thoughts on that?
>>
>> Yes, we have to somehow fix the crash and the assertion failure on
>> 9.1.
>
> Should the check_transaction_read_only() stuff be back-patched to
> 9.1, too? So far as we know, that's fragile, not broken, right?
> Could the fix you envision there cause a behavioral change that could
> break anything that users might have in place?

Good question. I don't see how it could cause a behavioral change, but
I've been wrong before.

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