Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> On 14.08.2012 14:25, Kevin Grittner wrote:
>> Heikki Linnakangas wrote:
>>> On 14.08.2012 08:23, Kevin Grittner wrote:
>> 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?
No, I wasn't getting errors in the clients or the logs, but I wasn't
seeing data pop up on the replica when I expected. Perhaps I messed
up my streaming replication configuration somehow. Do I understand
that you are now seeing correction of both bugs and no new
misbehaviors in your tests?
>>> 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.
> 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.
Yeah, I figured that out before posting version 2 of the patch. I
should have said something.
> Yet another option is to suggest the "BEGIN ISOLATION LEVEL
> REPEATABLE READ" syntax, instead of SET.
I'm inclined toward hinting at a session override of the default.
If you're typing away in psql, that's a lot less work. :-) That's
what I included in version 2 of the patch, but only if the default
was serializable. I did say to set it "before a transaction"
because a quick test showed that setting the default in a
transaction didn't keep that transaction from getting the error if
you proceeded to run a SELECT statement.
>>>> 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.
If we don't know of any actual existing bugs I'm inclined to not
back-patch that part to 9.1, although it makes sense for 9.2 since
we shouldn't be risking breakage of any production systems. I'm
really cautious about giving anybody any excuse not to apply a minor
update.
-Kevin