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


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

Reply via email to