Re: [HACKERS] Move WAL warning
On Thu, Feb 10, 2011 at 03:20, Fujii Masao wrote: > On Wed, Feb 9, 2011 at 7:02 PM, Magnus Hagander wrote: >> On Thu, Feb 3, 2011 at 11:19, Magnus Hagander wrote: >>> On Wed, Feb 2, 2011 at 18:00, Magnus Hagander wrote: On Wed, Feb 2, 2011 at 17:43, Heikki Linnakangas wrote: > On 02.02.2011 16:36, Magnus Hagander wrote: >> >> When running pg_basebackup with -x to include all transaction log, the >> server will still throw a warning about xlog archiving if it's not >> enabled - that is completely irrelevant since pg_basebackup has >> included it already (and if it was gone, the base backup step itself >> will fail - actual error and not warning). >> >> This patch moves the warning from do_pg_base_backup to pg_base_backup, >> so it still shows when using the explicit function calls, but goes >> away when using pg_basebackup. > > For the sake of consistency, how about moving the "pg_stop_backup > complete, > all required WAL segments have been archived" notice too? Well, it goes out as a NOTICE, so by default it doesn't show.. But yeah, for code-consistency it makes sense. Like so, then. >>> >>> Thinking some more about it, I realized this is not going to be enough >>> - we need to be able to turn off the waiting for WAL segment as well, >>> in the case when you're streaming the log. Thus, it needs to be >>> controllable from the backup client, and we can't just assume the >>> default is ok. >>> >>> Attached is an updated patch that adds a NOWAIT option to BASE_BACKUP, >>> that turns off the waiting. If it's set, it also doesn't warn about >>> not being able to wait in the case when there is nothing to wait for, >>> so this is a replacement for the previous patch. >> >> Applied. > > Back to your original complaint. When -x option is specified, pg_basebackup > should use NOWAIT option in BASE_BACKUP command to send to the server? Oh yeah, I put that in the wrong patch - it's in my patch to do the streaming wal. I'll extract it and apply it separately, in case the other stuff isn't finished. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Move WAL warning
On Wed, Feb 9, 2011 at 7:02 PM, Magnus Hagander wrote: > On Thu, Feb 3, 2011 at 11:19, Magnus Hagander wrote: >> On Wed, Feb 2, 2011 at 18:00, Magnus Hagander wrote: >>> On Wed, Feb 2, 2011 at 17:43, Heikki Linnakangas >>> wrote: On 02.02.2011 16:36, Magnus Hagander wrote: > > When running pg_basebackup with -x to include all transaction log, the > server will still throw a warning about xlog archiving if it's not > enabled - that is completely irrelevant since pg_basebackup has > included it already (and if it was gone, the base backup step itself > will fail - actual error and not warning). > > This patch moves the warning from do_pg_base_backup to pg_base_backup, > so it still shows when using the explicit function calls, but goes > away when using pg_basebackup. For the sake of consistency, how about moving the "pg_stop_backup complete, all required WAL segments have been archived" notice too? >>> >>> Well, it goes out as a NOTICE, so by default it doesn't show.. But >>> yeah, for code-consistency it makes sense. Like so, then. >> >> Thinking some more about it, I realized this is not going to be enough >> - we need to be able to turn off the waiting for WAL segment as well, >> in the case when you're streaming the log. Thus, it needs to be >> controllable from the backup client, and we can't just assume the >> default is ok. >> >> Attached is an updated patch that adds a NOWAIT option to BASE_BACKUP, >> that turns off the waiting. If it's set, it also doesn't warn about >> not being able to wait in the case when there is nothing to wait for, >> so this is a replacement for the previous patch. > > Applied. Back to your original complaint. When -x option is specified, pg_basebackup should use NOWAIT option in BASE_BACKUP command to send to the server? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Move WAL warning
On Thu, Feb 3, 2011 at 11:19, Magnus Hagander wrote: > On Wed, Feb 2, 2011 at 18:00, Magnus Hagander wrote: >> On Wed, Feb 2, 2011 at 17:43, Heikki Linnakangas >> wrote: >>> On 02.02.2011 16:36, Magnus Hagander wrote: When running pg_basebackup with -x to include all transaction log, the server will still throw a warning about xlog archiving if it's not enabled - that is completely irrelevant since pg_basebackup has included it already (and if it was gone, the base backup step itself will fail - actual error and not warning). This patch moves the warning from do_pg_base_backup to pg_base_backup, so it still shows when using the explicit function calls, but goes away when using pg_basebackup. >>> >>> For the sake of consistency, how about moving the "pg_stop_backup complete, >>> all required WAL segments have been archived" notice too? >> >> Well, it goes out as a NOTICE, so by default it doesn't show.. But >> yeah, for code-consistency it makes sense. Like so, then. > > Thinking some more about it, I realized this is not going to be enough > - we need to be able to turn off the waiting for WAL segment as well, > in the case when you're streaming the log. Thus, it needs to be > controllable from the backup client, and we can't just assume the > default is ok. > > Attached is an updated patch that adds a NOWAIT option to BASE_BACKUP, > that turns off the waiting. If it's set, it also doesn't warn about > not being able to wait in the case when there is nothing to wait for, > so this is a replacement for the previous patch. Applied. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Move WAL warning
On Wed, Feb 2, 2011 at 18:00, Magnus Hagander wrote:
> On Wed, Feb 2, 2011 at 17:43, Heikki Linnakangas
> wrote:
>> On 02.02.2011 16:36, Magnus Hagander wrote:
>>>
>>> When running pg_basebackup with -x to include all transaction log, the
>>> server will still throw a warning about xlog archiving if it's not
>>> enabled - that is completely irrelevant since pg_basebackup has
>>> included it already (and if it was gone, the base backup step itself
>>> will fail - actual error and not warning).
>>>
>>> This patch moves the warning from do_pg_base_backup to pg_base_backup,
>>> so it still shows when using the explicit function calls, but goes
>>> away when using pg_basebackup.
>>
>> For the sake of consistency, how about moving the "pg_stop_backup complete,
>> all required WAL segments have been archived" notice too?
>
> Well, it goes out as a NOTICE, so by default it doesn't show.. But
> yeah, for code-consistency it makes sense. Like so, then.
Thinking some more about it, I realized this is not going to be enough
- we need to be able to turn off the waiting for WAL segment as well,
in the case when you're streaming the log. Thus, it needs to be
controllable from the backup client, and we can't just assume the
default is ok.
Attached is an updated patch that adds a NOWAIT option to BASE_BACKUP,
that turns off the waiting. If it's set, it also doesn't warn about
not being able to wait in the case when there is nothing to wait for,
so this is a replacement for the previous patch.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 66cc004..a48e36c 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8615,7 +8615,7 @@ pg_stop_backup(PG_FUNCTION_ARGS)
XLogRecPtr stoppoint;
char stopxlogstr[MAXFNAMELEN];
- stoppoint = do_pg_stop_backup(NULL);
+ stoppoint = do_pg_stop_backup(NULL, true);
snprintf(stopxlogstr, sizeof(stopxlogstr), "%X/%X",
stoppoint.xlogid, stoppoint.xrecoff);
@@ -8630,7 +8630,7 @@ pg_stop_backup(PG_FUNCTION_ARGS)
* the non-exclusive backup specified by 'labelfile'.
*/
XLogRecPtr
-do_pg_stop_backup(char *labelfile)
+do_pg_stop_backup(char *labelfile, bool waitforarchive)
{
bool exclusive = (labelfile == NULL);
XLogRecPtr startpoint;
@@ -8829,7 +8829,7 @@ do_pg_stop_backup(char *labelfile)
* wish to wait, you can set statement_timeout. Also, some notices are
* issued to clue in anyone who might be doing this interactively.
*/
- if (XLogArchivingActive())
+ if (waitforarchive && XLogArchivingActive())
{
XLByteToPrevSeg(stoppoint, _logId, _logSeg);
XLogFileName(lastxlogfilename, ThisTimeLineID, _logId, _logSeg);
@@ -8870,7 +8870,7 @@ do_pg_stop_backup(char *labelfile)
ereport(NOTICE,
(errmsg("pg_stop_backup complete, all required WAL segments have been archived")));
}
- else
+ else if (waitforarchive)
ereport(NOTICE,
(errmsg("WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup")));
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 29284a6..0bd1fa0 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -37,6 +37,7 @@ typedef struct
const char *label;
bool progress;
bool fastcheckpoint;
+ bool nowait;
bool includewal;
} basebackup_options;
@@ -171,7 +172,7 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
}
PG_END_ENSURE_ERROR_CLEANUP(base_backup_cleanup, (Datum) 0);
- endptr = do_pg_stop_backup(labelfile);
+ endptr = do_pg_stop_backup(labelfile, !opt->nowait);
if (opt->includewal)
{
@@ -251,6 +252,7 @@ parse_basebackup_options(List *options, basebackup_options *opt)
bool o_label = false;
bool o_progress = false;
bool o_fast = false;
+ bool o_nowait = false;
bool o_wal = false;
MemSet(opt, 0, sizeof(*opt));
@@ -285,6 +287,15 @@ parse_basebackup_options(List *options, basebackup_options *opt)
opt->fastcheckpoint = true;
o_fast = true;
}
+ else if (strcmp(defel->defname, "nowait") == 0)
+ {
+ if (o_nowait)
+ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("duplicate option \"%s\"", defel->defname)));
+ opt->nowait = true;
+ o_nowait = true;
+ }
else if (strcmp(defel->defname, "wal") == 0)
{
if (o_wal)
diff --git a/src/backend/replication/repl_gram.y b/src/backend/replication/repl_gram.y
index e1a4a51..4930ad1 100644
--- a/src/backend/replication/repl_gram.y
+++ b/src/backend/replication/repl_gram.y
@@ -71,6 +71,7 @@ Node *replication_parse_result;
%token K_LABEL
%token K_PROGRESS
%token K_FAST
+%token K_NOWAIT
%token K_WAL
%token K_START_REPLICATION
@@ -107,7 +108,7 @@ identify_system:
;
/*
- * BASE_BACKUP [LABEL ''] [PROGRESS] [FAST] [WAL]
+ * BASE_BACKUP [LABEL ''] [PROGRESS] [FAST] [WAL]
Re: [HACKERS] Move WAL warning
On Wed, Feb 2, 2011 at 17:43, Heikki Linnakangas
wrote:
> On 02.02.2011 16:36, Magnus Hagander wrote:
>>
>> When running pg_basebackup with -x to include all transaction log, the
>> server will still throw a warning about xlog archiving if it's not
>> enabled - that is completely irrelevant since pg_basebackup has
>> included it already (and if it was gone, the base backup step itself
>> will fail - actual error and not warning).
>>
>> This patch moves the warning from do_pg_base_backup to pg_base_backup,
>> so it still shows when using the explicit function calls, but goes
>> away when using pg_basebackup.
>
> For the sake of consistency, how about moving the "pg_stop_backup complete,
> all required WAL segments have been archived" notice too?
Well, it goes out as a NOTICE, so by default it doesn't show.. But
yeah, for code-consistency it makes sense. Like so, then.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 66cc004..d7559b8 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8619,6 +8619,14 @@ pg_stop_backup(PG_FUNCTION_ARGS)
snprintf(stopxlogstr, sizeof(stopxlogstr), "%X/%X",
stoppoint.xlogid, stoppoint.xrecoff);
+
+ if (XLogArchivingActive())
+ ereport(NOTICE,
+(errmsg("pg_stop_backup complete, all required WAL segments have been archived")));
+ else
+ ereport(NOTICE,
+(errmsg("WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup")));
+
PG_RETURN_TEXT_P(cstring_to_text(stopxlogstr));
}
@@ -8866,13 +8874,7 @@ do_pg_stop_backup(char *labelfile)
"but the database backup will not be usable without all the WAL segments.")));
}
}
-
- ereport(NOTICE,
-(errmsg("pg_stop_backup complete, all required WAL segments have been archived")));
}
- else
- ereport(NOTICE,
-(errmsg("WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup")));
/*
* We're done. As a convenience, return the ending WAL location.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Move WAL warning
On 02.02.2011 16:36, Magnus Hagander wrote: When running pg_basebackup with -x to include all transaction log, the server will still throw a warning about xlog archiving if it's not enabled - that is completely irrelevant since pg_basebackup has included it already (and if it was gone, the base backup step itself will fail - actual error and not warning). This patch moves the warning from do_pg_base_backup to pg_base_backup, so it still shows when using the explicit function calls, but goes away when using pg_basebackup. For the sake of consistency, how about moving the "pg_stop_backup complete, all required WAL segments have been archived" notice too? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Move WAL warning
When running pg_basebackup with -x to include all transaction log, the
server will still throw a warning about xlog archiving if it's not
enabled - that is completely irrelevant since pg_basebackup has
included it already (and if it was gone, the base backup step itself
will fail - actual error and not warning).
This patch moves the warning from do_pg_base_backup to pg_base_backup,
so it still shows when using the explicit function calls, but goes
away when using pg_basebackup.
Objections?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 66cc004..7d1a95e 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8619,6 +8619,11 @@ pg_stop_backup(PG_FUNCTION_ARGS)
snprintf(stopxlogstr, sizeof(stopxlogstr), "%X/%X",
stoppoint.xlogid, stoppoint.xrecoff);
+
+ if (!XLogArchivingActive())
+ ereport(NOTICE,
+(errmsg("WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup")));
+
PG_RETURN_TEXT_P(cstring_to_text(stopxlogstr));
}
@@ -8870,9 +8875,6 @@ do_pg_stop_backup(char *labelfile)
ereport(NOTICE,
(errmsg("pg_stop_backup complete, all required WAL segments have been archived")));
}
- else
- ereport(NOTICE,
-(errmsg("WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup")));
/*
* We're done. As a convenience, return the ending WAL location.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
