On 17/10/14 06:25, Michael Paquier wrote:
Two votes in favor of that from two committers sounds like a deal. Here
is an refreshed version of the patch introducing --snapshot from here,
after fixing a couple of things and adding documentation:
http://www.postgresql.org/message-id/ca+u5nmk9+ttcff_-4mfdxwhnastauhuq7u7uedd57vay28a...@mail.gmail.com


Hi,

I have two minor things:

+ printf(_(" --snapshot use given synchronous snapshot for the dump\n"));

I think this should say --snapshot=NAME or something like that to make it visible that you are supposed to provide the parameter.

The other thing is, you changed logic of the parallel dump behavior a little - before your patch it works in a way that one connection exports snapshot and others do "SET TRANSACTION SNAPSHOT", after your patch, even the connection that exported the snapshot does the "SET TRANSACTION SNAPSHOT" which is unnecessary. I don't see it as too big deal but I don't see the need to change that behavior either.

You could do something like this instead maybe?:
if (AH->sync_snapshot_id)
    SET TRANSACTION SNAPSHOT
else if (AH->numWorkers > 1 && AH->remoteVersion >= 90200 && !dopt->no_synchronized_snapshots)
    AH->sync_snapshot_id = get_synchronized_snapshot(AH);

And remove the else if for the if (dumpsnapshot) part.

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


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