From: David Steele <david@pgmasters.net>
To: Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>
Cc: Michael Paquier <michael.paquier@gmail.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Cc: Stephen Frost <sfrost@snowman.net>
Subject: Re: PATCH: Make pg_stop_backup() archive wait optional
Date: Fri, 17 Mar 2017 10:31:56 -0400
Message-ID: <49d70e8e-33bf-2390-e717-ba9562621aa4@pgmasters.net> (raw)
In-Reply-To: <0A3221C70F24FB45833433255569204D1F6B5134@G01JPEXMBYT05>
References: <758e3fd1-45b4-5e28-75cd-e9e7f93a4c02@pgmasters.net>
<CAB7nPqRf6LnNN3TZZGi5DJ8sTWE5XekBSAj7E0CqsJ1nDGhiEA@mail.gmail.com>
<35b7530c-68ce-7090-e13c-e39e4dac9bca@pgmasters.net>
<CAB7nPqS-vH_Hk=+n63SHKS4ZrpknwNN9KMr9WkPZ9De=wU02Yg@mail.gmail.com>
<967b90bd-6b65-a18a-5266-bfa9191c40e6@pgmasters.net>
<CA+TgmoYCWfO2UM-t=HUMFJyxJywLDiLL0nAJpx88LKtvBvNECw@mail.gmail.com>
<aa775a6f-059c-47dc-ef87-1f017073014b@pgmasters.net>
<CA+TgmobfcE=fVoqhgWr6SFuhARdo6DY2Yz27eOevJnXr0ch1Lw@mail.gmail.com>
<0f46c90e-2884-59be-69bf-8f467d4c9ba2@pgmasters.net>
<CA+TgmoZfjF2k6N8Rfga20bXxnRAttuz8N6dvxeOu=t3j0FYYsg@mail.gmail.com>
<081cbf08-35c1-4580-8c31-3ae70b507fb1@pgmasters.net>
<0A3221C70F24FB45833433255569204D1F6B3F95@G01JPEXMBYT05>
<0A3221C70F24FB45833433255569204D1F6B5134@G01JPEXMBYT05>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
On 3/17/17 4:18 AM, Tsunakawa, Takayuki wrote:
> From: pgsql-hackers-owner@postgresql.org
>> [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Tsunakawa,
>> Takayuki
>> I made this ready for committer. The patch applied except for catversion.h,
>> the patch content looks good, and the target test passed as follows:
>
> Sorry, I reverted this to waiting for author, because make check failed. src/test/regress/expected/opr_sanity.out seems to need revision because pg_stop_backup() was added.
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.
Thanks,
--
-David
david@pgmasters.net
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgmlindex 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);
</row>
<row>
<entry>
- <literal><function>pg_stop_backup(<parameter>exclusive</> <type>boolean</>)</function></literal>+ <literal><function>pg_stop_backup(<parameter>exclusive</> <type>boolean</> <optional>, <parameter>wait_for_archive</> <type>boolean</> </optional>)</function></literal>
</entry>
<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.
</para>
<para>
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.cindex 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 backup
* 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 label
* and tablespace map so they can be written to disk by the caller.
*/
- 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;
cancel_before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sqlindex 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'
PARALLEL RESTRICTED;
+CREATE OR REPLACE FUNCTION pg_stop_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'+ PARALLEL RESTRICTED;+
-- legacy definition for compatibility with 9.3
CREATE OR REPLACE FUNCTION
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.hindex 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:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:
[text/plain] pgstopbackup-wait-v4.patch (5.5K, ../49d70e8e-33bf-2390-e717-ba9562621aa4@pgmasters.net/2-pgstopbackup-wait-v4.patch)
download | inline diff:diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgmlindex 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);
</row>
<row>
<entry>
- <literal><function>pg_stop_backup(<parameter>exclusive</> <type>boolean</>)</function></literal>+ <literal><function>pg_stop_backup(<parameter>exclusive</> <type>boolean</> <optional>, <parameter>wait_for_archive</> <type>boolean</> </optional>)</function></literal>
</entry>
<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.
</para>
<para>
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.cindex 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 backup
* 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 label
* and tablespace map so they can be written to disk by the caller.
*/
- 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;
cancel_before_shmem_exit(nonexclusive_base_backup_cleanup, (Datum) 0);
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sqlindex 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'
PARALLEL RESTRICTED;
+CREATE OR REPLACE FUNCTION pg_stop_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'+ PARALLEL RESTRICTED;+
-- legacy definition for compatibility with 9.3
CREATE OR REPLACE FUNCTION
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.hindex 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");
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: david@pgmasters.net, tsunakawa.takay@jp.fujitsu.com, michael.paquier@gmail.com, sfrost@snowman.net
Subject: Re: PATCH: Make pg_stop_backup() archive wait optional
In-Reply-To: <49d70e8e-33bf-2390-e717-ba9562621aa4@pgmasters.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox