On Mon, Oct 27, 2014 at 7:37 PM, Petr Jelinek <p...@2ndquadrant.com> wrote:
> I have two minor things:
> + printf(_(" --snapshot use given synchronous
> snapshot for the dump\n"));
Thanks for your input!
> I think this should say --snapshot=NAME or something like that to make it
> visible that you are supposed to provide the parameter.
Right. Let's do --snapshot=SNAPSHOT then.
> 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.
Indeed, let's save some resources and not have the connection
exporting the snapshot use SET TRANSACTION SNAPSHOT if that's not
necessary.
> 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.
Right as well. We still need to check for dumpsnapshot to set up
sync_snapshot_id for the first connection such as it can pass the
value to the other workers though.
Updated patch with those comments addressed is attached.
Regards,
--
Michael
From e4366fb02aa2f844f5482d221fe6544a09944c8c Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Fri, 17 Oct 2014 13:21:32 +0900
Subject: [PATCH] Add option --snapshot to pg_dump
This can be used to define a snapshot previously defined by a session
in parallel that has either used pg_export_snapshot or obtained one when
creating a logical slot. When this option is used with parallel pg_dump,
the snapshot defined by this option is used and no new snapshot is taken.
---
doc/src/sgml/ref/pg_dump.sgml | 20 +++++++++++++++++
src/bin/pg_dump/pg_dump.c | 51 ++++++++++++++++++++++++++++---------------
2 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index c92c6ee..f3ab71a 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -848,6 +848,26 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--snapshot=<replaceable class="parameter">snapshotname</replaceable></option></term>
+ <listitem>
+ <para>
+ Use given synchronous snapshot when taking a dump of the database (see
+ <xref linkend="functions-snapshot-synchronization-table"> for more
+ details).
+ </para>
+ <para>
+ This option is useful when needing a dump consistent with a session
+ in parallel that exported this snapshot, when for example creating
+ a logical replication slot (see <xref linkend="logicaldecoding">).
+ </para>
+ <para>
+ In the case of a parallel dump, the snapshot name defined by this
+ option is used in priority.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>--serializable-deferrable</option></term>
<listitem>
<para>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 1e8f089..2dd2bc4 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -126,7 +126,8 @@ static const CatalogId nilCatalogId = {0, 0};
static void help(const char *progname);
static void setup_connection(Archive *AH, DumpOptions *dopt,
- const char *dumpencoding, char *use_role);
+ const char *dumpencoding, const char *dumpsnapshot,
+ char *use_role);
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
@@ -269,6 +270,7 @@ main(int argc, char **argv)
RestoreOptions *ropt;
Archive *fout; /* the script file */
const char *dumpencoding = NULL;
+ const char *dumpsnapshot = NULL;
char *use_role = NULL;
int numWorkers = 1;
trivalue prompt_password = TRI_DEFAULT;
@@ -329,6 +331,7 @@ main(int argc, char **argv)
{"role", required_argument, NULL, 3},
{"section", required_argument, NULL, 5},
{"serializable-deferrable", no_argument, &dopt->serializable_deferrable, 1},
+ {"snapshot", required_argument, NULL, 6},
{"use-set-session-authorization", no_argument, &dopt->use_setsessauth, 1},
{"no-security-labels", no_argument, &dopt->no_security_labels, 1},
{"no-synchronized-snapshots", no_argument, &dopt->no_synchronized_snapshots, 1},
@@ -506,6 +509,10 @@ main(int argc, char **argv)
set_dump_section(optarg, &dopt->dumpSections);
break;
+ case 6: /* snapshot */
+ dumpsnapshot = pg_strdup(optarg);
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -614,7 +621,7 @@ main(int argc, char **argv)
* death.
*/
ConnectDatabase(fout, dopt->dbname, dopt->pghost, dopt->pgport, dopt->username, prompt_password);
- setup_connection(fout, dopt, dumpencoding, use_role);
+ setup_connection(fout, dopt, dumpencoding, dumpsnapshot, use_role);
/*
* Disable security label support if server version < v9.1.x (prevents
@@ -658,6 +665,11 @@ main(int argc, char **argv)
"Run with --no-synchronized-snapshots instead if you do not need\n"
"synchronized snapshots.\n");
+ /* check the version when a snapshot is explicitely specified by user */
+ if (dumpsnapshot && fout->remoteVersion < 90200)
+ exit_horribly(NULL,
+ "Exported snapshots are not supported by this server version.\n");
+
/* Find the last built-in OID, if needed */
if (fout->remoteVersion < 70300)
{
@@ -888,6 +900,7 @@ help(const char *progname)
printf(_(" --quote-all-identifiers quote all identifiers, even if not key words\n"));
printf(_(" --section=SECTION dump named section (pre-data, data, or post-data)\n"));
printf(_(" --serializable-deferrable wait until the dump can run without anomalies\n"));
+ printf(_(" --snapshot=SNAPSHOT use given synchronous snapshot for the dump\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
@@ -907,7 +920,8 @@ help(const char *progname)
}
static void
-setup_connection(Archive *AH, DumpOptions *dopt, const char *dumpencoding, char *use_role)
+setup_connection(Archive *AH, DumpOptions *dopt, const char *dumpencoding,
+ const char *dumpsnapshot, char *use_role)
{
PGconn *conn = GetConnection(AH);
const char *std_strings;
@@ -1015,22 +1029,25 @@ setup_connection(Archive *AH, DumpOptions *dopt, const char *dumpencoding, char
ExecuteSqlStatement(AH,
"SET TRANSACTION ISOLATION LEVEL SERIALIZABLE");
+ /*
+ * define an export snapshot, either chosen by user or needed for
+ * parallel dump.
+ */
+ if (dumpsnapshot)
+ AH->sync_snapshot_id = strdup(dumpsnapshot);
-
- if (AH->numWorkers > 1 && AH->remoteVersion >= 90200 && !dopt->no_synchronized_snapshots)
+ if (AH->sync_snapshot_id)
{
- if (AH->sync_snapshot_id)
- {
- PQExpBuffer query = createPQExpBuffer();
-
- appendPQExpBufferStr(query, "SET TRANSACTION SNAPSHOT ");
- appendStringLiteralConn(query, AH->sync_snapshot_id, conn);
- ExecuteSqlStatement(AH, query->data);
- destroyPQExpBuffer(query);
- }
- else
- AH->sync_snapshot_id = get_synchronized_snapshot(AH);
+ PQExpBuffer query = createPQExpBuffer();
+ appendPQExpBuffer(query, "SET TRANSACTION SNAPSHOT ");
+ appendStringLiteralConn(query, AH->sync_snapshot_id, conn);
+ ExecuteSqlStatement(AH, query->data);
+ destroyPQExpBuffer(query);
}
+ else if (AH->numWorkers > 1 &&
+ AH->remoteVersion >= 90200 &&
+ !dopt->no_synchronized_snapshots)
+ AH->sync_snapshot_id = get_synchronized_snapshot(AH);
if (AH->remoteVersion >= 90500)
{
@@ -1044,7 +1061,7 @@ setup_connection(Archive *AH, DumpOptions *dopt, const char *dumpencoding, char
static void
setupDumpWorker(Archive *AHX, DumpOptions *dopt, RestoreOptions *ropt)
{
- setup_connection(AHX, dopt, NULL, NULL);
+ setup_connection(AHX, dopt, NULL, NULL, NULL);
}
static char *
--
2.1.2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
