From: | Petr Jelinek <petr(at)2ndquadrant(dot)com> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)2ndquadrant(dot)com>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Bernd Helmle <mailings(at)oopsware(dot)de>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Better support of exported snapshots with pg_dump |
Date: | 2014-10-27 10:37:34 |
Message-ID: | 544E206E.3010302@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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_-4MfdxWHnASTAuHuq7u7uedD57vaY28AsQA@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
From | Date | Subject | |
---|---|---|---|
Next Message | Abhijit Menon-Sen | 2014-10-27 10:39:30 | Re: END_OF_RECOVERY shutdowns and ResetUnloggedRelations() |
Previous Message | Ali Akbar | 2014-10-27 10:20:43 | Re: Function array_agg(array) |