Обсуждение: pg_dump with both --serializable-deferrable and -j

Alexander Korotkov <aekorotkov@gmail.com> wrote:

> when pg_dump is run with both --serializable-deferrable and -j
> options to pg_dump, it returns errors:
>
> pg_dump: [archiver (db)] query failed: ERROR:  a snapshot-importing transaction must not be READ ONLY DEFERRABLE
> pg_dump: [archiver (db)] query failed: ERROR:  a snapshot-importing transaction must not be READ ONLY DEFERRABLE
> pg_dump: [archiver (db)] query failed: ERROR:  a snapshot-importing transaction must not be READ ONLY DEFERRABLE
> pg_dump: [parallel archiver] query was: SET TRANSACTION SNAPSHOT '0001E300-1'
> pg_dump: [archiver (db)] query failed: ERROR:  a snapshot-importing transaction must not be READ ONLY DEFERRABLE
>
> I've checked it on 9.4.0 and 9.3.5.
>
> So, these options are incompatible now.

The only way to make them compatible would be to have some way for
an exported snapshot to indicate that it is safe against
serialization anomalies if used for a serializable transaction, and
to only generate an error if a non-safe snapshot is used with the
--serializable-deferrable option.

> Could we start snapshot-importing transaction with repeatable
> read isolation level?

You can if you don't use the option which specifies that you want
serializable behavior.  Why specify --serializable-deferrable if
you don't?

> AFAICS, they should read exactly same data as snapshot-exporting
> serializable transaction.

Sort of.  The behavior once they have a snapshot and are running is
the same; the difference is whether the snapshot can see a
transient state which would not be consistent with some serial
order of transaction execution.  For examples, see:

https://wiki.postgresql.org/wiki/SSI#Read_Only_Transactions

It's really a question of why you want the dump.  If it is for
crash recovery, you don't need --serializable-deferrable.  If you
want to restore it to run reports that only show states consistent
with some serial execution of serializable transactions you *do*
need --serializable-deferrable, so that the snapshot used for the
dump reflects a point in the commit stream when no serialization
anomalies can be visible to a read-only transaction.

> If not, could pg_dump return some more friendly error before
> actually trying to dump?

Sure, until such time as it is possible to request a
serializable-safe snapshot and flag it as such on export (and check
for that in pg_dump), we could add a validation that these two
options are incompatible.  If there are no objections, I can push
something to do that.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Kevin Grittner <kgrittn@ymail.com> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> wrote:

>> Could we start snapshot-importing transaction with repeatable
>> read isolation level?
>
> You can if you don't use the option which specifies that you want
> serializable behavior.  Why specify --serializable-deferrable if
> you don't?
>
>> AFAICS, they should read exactly same data as snapshot-exporting
>> serializable transaction.
>
> Sort of.  The behavior once they have a snapshot and are running is
> the same; the difference is whether the snapshot can see a
> transient state which would not be consistent with some serial
> order of transaction execution.

Oh, wait; on a re-read I think I may have misunderstood the question.

If you are talking about having pg_dump acquire a safe snapshot and
have cooperating processes in the same pg_dump run use that
snapshot in repeatable read transactions, then yes -- that would
work.  As long as a repeatable read transaction is using a safe
snapshot it will not see any anomalies.  That would be a better
solution if it can be done.  Do you have any code to suggest, or
should I look at writing it?

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2015-01-28 14:54:15 +0000, Kevin Grittner wrote:
> Kevin Grittner <kgrittn@ymail.com> wrote:
> > Alexander Korotkov <aekorotkov@gmail.com> wrote:
> 
> >> Could we start snapshot-importing transaction with repeatable
> >> read isolation level?
> >
> > You can if you don't use the option which specifies that you want
> > serializable behavior.  Why specify --serializable-deferrable if
> > you don't?
> >
> >> AFAICS, they should read exactly same data as snapshot-exporting
> >> serializable transaction.
> >
> > Sort of.  The behavior once they have a snapshot and are running is
> > the same; the difference is whether the snapshot can see a
> > transient state which would not be consistent with some serial
> > order of transaction execution.
> 
> Oh, wait; on a re-read I think I may have misunderstood the question.
> 
> If you are talking about having pg_dump acquire a safe snapshot and
> have cooperating processes in the same pg_dump run use that
> snapshot in repeatable read transactions, then yes -- that would
> work.  As long as a repeatable read transaction is using a safe
> snapshot it will not see any anomalies.  That would be a better
> solution if it can be done.  Do you have any code to suggest, or
> should I look at writing it?

Why could it be unsafe to import a snapshot that's been generated as
serializable deferrable into another backend? Doesn't the fact that it
has been exported out of a deferrable xact that's still running pretty
much guarantee that the other xact is also safe?

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.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

Andres Freund <andres@2ndquadrant.com> wrote:
> Kevin Grittner <kgrittn@ymail.com> wrote:

>>> Alexander Korotkov <aekorotkov@gmail.com> wrote:
>>>
>>>> Could we start snapshot-importing transaction with repeatable
>>>> read isolation level?

>> If you are talking about having pg_dump acquire a safe snapshot and
>> have cooperating processes in the same pg_dump run use that
>> snapshot in repeatable read transactions, then yes -- that would
>> work.  As long as a repeatable read transaction is using a safe
>> snapshot it will not see any anomalies.

> Why could it be unsafe to import a snapshot that's been generated as
> serializable deferrable into another backend?

That wouldn't be unsafe, which is what I was saying the post you
responded to.

> Doesn't the fact that it
> has been exported out of a deferrable xact that's still running pretty
> much guarantee that the other xact is also safe?

As long as it is at least repeatable read, yes.

> 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.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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?


--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

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.
        regards, tom lane

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

Kevin Grittner <kgrittn@ymail.com> wrote:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:

>> A comment seems essential here, because as written anybody would
>> think the test for a snapshot is a bug.
>
> Good point.

I propose to apply the attached to master and back-patch to 9.3,
and follow that with a patch (for master only) along the lines
suggested by Andres.  Since *that* change is more invasive and
changes existing behavior I will submit it to the open CF for
review.  Objections?

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Kevin Grittner <kgrittn@ymail.com> writes:
> I propose to apply the attached to master and back-patch to 9.3, 
> and follow that with a patch (for master only) along the lines 
> suggested by Andres.  Since *that* change is more invasive and 
> changes existing behavior I will submit it to the open CF for 
> review.  Objections?

Only the nit-picky one that I quite dislike putting a comment block inside
an if-condition like that.  It's not really house style around here,
and in particular I suspect pgindent might not treat it nicely.
        regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Kevin Grittner <kgrittn@ymail.com> writes:

>> I propose to apply the attached to master and back-patch to 9.3

>> Objections?
>
> Only the nit-picky one that I quite dislike putting a comment 
> block inside an if-condition like that.

Comment moved above the if-condition, and pushed.

Thanks for the report, Alexander!

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company