On 3/21/17 2:34 PM, Fujii Masao wrote:
On Tue, Mar 21, 2017 at 11:03 AM, Tsunakawa, Takayuki
<tsunakawa.ta...@jp.fujitsu.com> wrote:
From: David Steele [mailto:da...@pgmasters.net]
Well, that's embarrassing.  When I recreated the function to add defaults
I messed up the AS clause and did not pay attention to the results of the
regression tests, apparently.

Attached is a new version rebased on 88e66d1.  Catalog version bump has
also been omitted.

I was late to reply because yesterday was a national holiday in Japan.  I 
marked this as ready for committer.  The patch applied cleanly and worked as 

The patch basically looks good to me, but one comment is;
backup.sgml (at least the description for "Making a non-exclusive
low level backup) seems to need to be updated.

Agreed.  Added in the attached patch and rebased on 8027556.

diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 2d67521..c04eb46 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -887,7 +887,7 @@ SELECT pg_start_backup('label', false, false);
      In the same connection as before, issue the command:
-SELECT * FROM pg_stop_backup(false);
+SELECT * FROM pg_stop_backup(false [, true ]);
      This terminates the backup mode and performs an automatic switch to
      the next WAL segment.  The reason for the switch is to arrange for
@@ -895,6 +895,15 @@ SELECT * FROM pg_stop_backup(false);
      ready to archive.
+     If the backup process monitors the WAL archiving process independently,
+     the second parameter (which defaults to true) can be set to false to
+     prevent <function>pg_stop_backup</> from blocking until all WAL is
+     archived.  Instead, the function will return as soon as the stop backup
+     record is written to the WAL.  This option must be used with caution:
+     if WAL archiving is not monitored correctly then the result might be a
+     useless backup.
+    </para>
+    <para>
      The <function>pg_stop_backup</> will return one row with three
      values. The second of these fields should be written to a file named
      <filename>backup_label</> in the root directory of the backup. The
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9408a25..4dc30ca 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18355,7 +18355,7 @@ SELECT set_config('log_statement_stats', 'off', false);
-        <literal><function>pg_stop_backup(<parameter>exclusive</> 
+        <literal><function>pg_stop_backup(<parameter>exclusive</> 
<type>boolean</> <optional>, <parameter>wait_for_archive</> <type>boolean</> 
        <entry><type>setof record</type></entry>
        <entry>Finish performing exclusive or non-exclusive on-line backup 
(restricted to superusers by default, but other users can be granted EXECUTE to 
run the function)</entry>
@@ -18439,7 +18439,13 @@ postgres=# select pg_start_backup('label_goes_here');
     <function>pg_start_backup</>. In a non-exclusive backup, the contents of
     the <filename>backup_label</> and <filename>tablespace_map</> are returned
     in the result of the function, and should be written to files in the
-    backup (and not in the data directory).
+    backup (and not in the data directory).  There is an optional second
+    parameter of type boolean.  If false, the <function>pg_stop_backup</>
+    will return immediately after the backup is completed without waiting for
+    WAL to be archived.  This behavior is only useful for backup
+    software which independently monitors WAL archiving. Otherwise, WAL
+    required to make the backup consistent might be missing and make the backup
+    useless.
diff --git a/src/backend/access/transam/xlogfuncs.c 
index 96aa15e..1ef9f2b 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -190,6 +190,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
        bool            nulls[3];
        bool            exclusive = PG_GETARG_BOOL(0);
+       bool            wait_for_archive = PG_GETARG_BOOL(1);
        XLogRecPtr      stoppoint;
        /* check to see if caller supports us returning a tuplestore */
@@ -232,7 +233,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
                 * Stop the exclusive backup, and since we're in an exclusive 
                 * return NULL for both backup_label and tablespace_map.
-               stoppoint = do_pg_stop_backup(NULL, true, NULL);
+               stoppoint = do_pg_stop_backup(NULL, wait_for_archive, NULL);
                exclusive_backup_running = false;
                nulls[1] = true;
@@ -250,7 +251,7 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
                 * Stop the non-exclusive backup. Return a copy of the backup 
                 * and tablespace map so they can be written to disk by the 
-               stoppoint = do_pg_stop_backup(label_file->data, true, NULL);
+               stoppoint = do_pg_stop_backup(label_file->data, 
wait_for_archive, NULL);
                nonexclusive_backup_running = false;
(Datum) 0);
diff --git a/src/backend/catalog/system_views.sql 
index b6552da..c2b0bed 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -988,6 +988,12 @@ CREATE OR REPLACE FUNCTION
   RETURNS pg_lsn STRICT VOLATILE LANGUAGE internal AS 'pg_start_backup'
+        exclusive boolean, wait_for_archive boolean DEFAULT true,
+        OUT lsn pg_lsn, OUT labelfile text, OUT spcmapfile text)
+  RETURNS SETOF record STRICT VOLATILE LANGUAGE internal as 'pg_stop_backup_v2'
 -- legacy definition for compatibility with 9.3
   json_populate_record(base anyelement, from_json json, use_json_as_text 
boolean DEFAULT false)
@@ -1088,7 +1094,7 @@ AS 'jsonb_insert';
 -- available to superuser / cluster owner, if they choose.
 REVOKE EXECUTE ON FUNCTION pg_start_backup(text, boolean, boolean) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_stop_backup() FROM public;
-REVOKE EXECUTE ON FUNCTION pg_stop_backup(boolean) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_stop_backup(boolean, boolean) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_create_restore_point(text) FROM public;
 REVOKE EXECUTE ON FUNCTION pg_switch_wal() FROM public;
 REVOKE EXECUTE ON FUNCTION pg_wal_replay_pause() FROM public;
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 836d6ff..2263565 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3172,7 +3172,7 @@ DATA(insert OID = 2172 ( pg_start_backup          PGNSP 
PGUID 12 1 0 0 0 f f f f t f v r
 DESCR("prepare for taking an online backup");
 DATA(insert OID = 2173 ( pg_stop_backup                        PGNSP PGUID 12 
1 0 0 0 f f f f t f v r 0 0 3220 "" _null_ _null_ _null_ _null_ _null_ 
pg_stop_backup _null_ _null_ _null_ ));
 DESCR("finish taking an online backup");
-DATA(insert OID = 2739 ( pg_stop_backup                        PGNSP PGUID 12 
1 1 0 0 f f f f t t v r 1 0 2249 "16" "{16,3220,25,25}" "{i,o,o,o}" 
"{exclusive,lsn,labelfile,spcmapfile}" _null_ _null_ pg_stop_backup_v2 _null_ 
_null_ _null_ ));
+DATA(insert OID = 2739 ( pg_stop_backup                        PGNSP PGUID 12 
1 1 0 0 f f f f t t v r 2 0 2249 "16 16" "{16,16,3220,25,25}" "{i,i,o,o,o}" 
"{exclusive,wait_for_archive,lsn,labelfile,spcmapfile}" _null_ _null_ 
pg_stop_backup_v2 _null_ _null_ _null_ ));
 DESCR("finish taking an online backup");
 DATA(insert OID = 3813 ( pg_is_in_backup               PGNSP PGUID 12 1 0 0 0 
f f f f t f v s 0 0 16 "" _null_ _null_ _null_ _null_ _null_ pg_is_in_backup 
_null_ _null_ _null_ ));
 DESCR("true if server is in online backup");
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to