Andres Freund <andres@2ndquadrant.com> wrote:
> On 2015-01-28 15:32:15 +0000, Kevin Grittner wrote:
>> Andres Freund <andres@2ndquadrant.com> wrote:
>>> ISTM that the check is just overzelous and/or needs to be moved into
>>> ImportSnapshot(). There it then could be made to check if the exporting
>>> xact was also deferrable.
>>
>> That would be great if ImportSnapshot had access to that
>> information; I don't see it, though. Having pg_dump use repeatable
>> read transactions for the processes that import the snapshot would
>> work fine, as long as they are reading a snapshot which was
>> captured by a serializable read only deferrable transaction.
>
> Then add that information? The disk format for snapshot isn't persistent
> across restarts, so we can just extend it.
>
> I really don't like adding hacks like using a lower serializability
> level than what's actually requested just because it happens to be
> easier. Even if it's just in some backend.
I see your point, and will look at that for the master branch, but
it hardly seems like something to back-patch; and the messy failure
of this combination of options on 9.3 and 9.4 seems like it
deserves a fix.
Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Kevin Grittner <kgrittn@ymail.com> writes:
>>
>> Kevin Grittner <kgrittn@ymail.com> wrote:
>>> Having pg_dump use repeatable read transactions for the processes
>>> that import the snapshot would work fine, as long as they are
>>> reading a snapshot which was captured by a serializable read only
>>> deferrable transaction.
>>
>> It looks like the attached patch does it (although it is only
>> lightly tested so far and only on the master branch). This seems
>> like a back-patchable bug fix (to 9.3).
>>
>> Thoughts?
>
> A comment seems essential here, because as written anybody would think
> the test for a snapshot is a bug.
Good point.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company