On Fri, Oct 17, 2014 at 12:19 AM, Robert Haas <robertmh...@gmail.com> wrote:

> On Wed, Oct 15, 2014 at 1:06 AM, Andres Freund <and...@2ndquadrant.com>
> wrote:
> > I personally think we should just disregard the race here and add a
> > snapshot parameter. The race is already there and not exactly
> > small. Let's not kid ourselves that hiding it solves anything.
> I, too, favor that plan.
>
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

When the snapshot specified by user is not a valid one, here is the error
returned by pg_dump:
$ pg_dump --snapshot 'ppo'
pg_dump: [archiver (db)] query failed: ERROR:  invalid snapshot identifier:
"ppo"
pg_dump: [archiver (db)] query was: SET TRANSACTION SNAPSHOT 'ppo'
I thinks that's fine, and it makes the code lighter to rely on the existing
error machinery.

Regards,
-- 
Michael
From 6f0d5370d152fc7979632a0abc1dee1ac58f8b30 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     | 52 +++++++++++++++++++++++++++++--------------
 2 files changed, 55 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 c56a4cb..2d3c7dd 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                   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,21 +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);
+	else if (AH->numWorkers > 1 &&
+			 AH->remoteVersion >= 90200 &&
+			 !dopt->no_synchronized_snapshots &&
+			 AH->sync_snapshot_id == NULL)
+		AH->sync_snapshot_id = get_synchronized_snapshot(AH);
 
-
-	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);
 	}
 
 	if (AH->remoteVersion >= 90500)
@@ -1044,7 +1062,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

Reply via email to