pg_basebackup and pg_stat_tmp directory
Hi,
The files in pg_stat_tmp directory don't need to be backed up because they are
basically reset at the archive recovery. So I think it's worth
changing pg_basebackup
so that it skips any files in pg_stat_tmp directory. Thought?
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Jan 28, 2014 at 12:56 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
Hi,
The files in pg_stat_tmp directory don't need to be backed up because they are
basically reset at the archive recovery. So I think it's worth
changing pg_basebackup
so that it skips any files in pg_stat_tmp directory. Thought?
Skipping pgstat_temp_directory in basebackup.c would make more sense
than directly touching pg_basebackup.
My 2c.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Jan 28, 2014 at 1:08 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Tue, Jan 28, 2014 at 12:56 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
Hi,
The files in pg_stat_tmp directory don't need to be backed up because they are
basically reset at the archive recovery. So I think it's worth
changing pg_basebackup
so that it skips any files in pg_stat_tmp directory. Thought?Skipping pgstat_temp_directory in basebackup.c would make more sense
than directly touching pg_basebackup.
My 2c.
Yeah, that's what I was thinking :)
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Jan 28, 2014 at 9:26 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
Hi,
The files in pg_stat_tmp directory don't need to be backed up because they are
basically reset at the archive recovery. So I think it's worth
changing pg_basebackup
so that it skips any files in pg_stat_tmp directory. Thought?
I think this is good idea, but can't it also avoid
PGSTAT_STAT_PERMANENT_TMPFILE along with temp files in
pg_stat_tmp
With Regards,
Amit Kapila.
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
On Tue, Jan 28, 2014 at 6:11 AM, Amit Kapila <amit.kapila16@gmail.com>wrote:
On Tue, Jan 28, 2014 at 9:26 AM, Fujii Masao <masao.fujii@gmail.com>
wrote:Hi,
The files in pg_stat_tmp directory don't need to be backed up because
they are
basically reset at the archive recovery. So I think it's worth
changing pg_basebackup
so that it skips any files in pg_stat_tmp directory. Thought?I think this is good idea, but can't it also avoid
PGSTAT_STAT_PERMANENT_TMPFILE along with temp files in
pg_stat_tmp
All stats files should be excluded. IIRC the PGSTAT_STAT_PERMANENT_TMPFILE
refers to just the global one. You want to exclude based
on PGSTAT_STAT_PERMANENT_DIRECTORY (and of course based on the
guc stats_temp_directory if it's in PGDATA.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
On Tue, Jan 28, 2014 at 5:51 PM, Magnus Hagander <magnus@hagander.net> wrote:
On Tue, Jan 28, 2014 at 6:11 AM, Amit Kapila <amit.kapila16@gmail.com>
wrote:On Tue, Jan 28, 2014 at 9:26 AM, Fujii Masao <masao.fujii@gmail.com>
wrote:Hi,
The files in pg_stat_tmp directory don't need to be backed up because
they are
basically reset at the archive recovery. So I think it's worth
changing pg_basebackup
so that it skips any files in pg_stat_tmp directory. Thought?I think this is good idea, but can't it also avoid
PGSTAT_STAT_PERMANENT_TMPFILE along with temp files in
pg_stat_tmpAll stats files should be excluded. IIRC the PGSTAT_STAT_PERMANENT_TMPFILE
refers to just the global one. You want to exclude based on
PGSTAT_STAT_PERMANENT_DIRECTORY (and of course based on the guc
stats_temp_directory if it's in PGDATA.
Attached patch changes basebackup.c so that it skips all files in both
pg_stat_tmp
and stats_temp_directory. Even when a user sets stats_temp_directory
to the directory
other than pg_stat_tmp, we need to skip the files in pg_stat_tmp. Because,
per recent change of pg_stat_statements, the external query file is
always created there.
Regards,
--
Fujii Masao
Attachments:
basebackup_skips_temp_stat_files_v1.patchtext/x-patch; charset=US-ASCII; name=basebackup_skips_temp_stat_files_v1.patchDownload
*** a/contrib/pg_stat_statements/pg_stat_statements.c
--- b/contrib/pg_stat_statements/pg_stat_statements.c
***************
*** 68,73 ****
--- 68,74 ----
#include "parser/analyze.h"
#include "parser/parsetree.h"
#include "parser/scanner.h"
+ #include "pgstat.h"
#include "storage/fd.h"
#include "storage/ipc.h"
#include "storage/spin.h"
***************
*** 89,95 **** PG_MODULE_MAGIC;
* race conditions. Besides, we only expect modest, infrequent I/O for query
* strings, so placing the file on a faster filesystem is not compelling.
*/
! #define PGSS_TEXT_FILE "pg_stat_tmp/pgss_query_texts.stat"
/* Magic number identifying the stats file format */
static const uint32 PGSS_FILE_HEADER = 0x20140125;
--- 90,96 ----
* race conditions. Besides, we only expect modest, infrequent I/O for query
* strings, so placing the file on a faster filesystem is not compelling.
*/
! #define PGSS_TEXT_FILE PG_STAT_TMP_DIR "/pgss_query_texts.stat"
/* Magic number identifying the stats file format */
static const uint32 PGSS_FILE_HEADER = 0x20140125;
*** a/src/backend/replication/basebackup.c
--- b/src/backend/replication/basebackup.c
***************
*** 25,30 ****
--- 25,31 ----
#include "libpq/pqformat.h"
#include "miscadmin.h"
#include "nodes/pg_list.h"
+ #include "pgstat.h"
#include "replication/basebackup.h"
#include "replication/walsender.h"
#include "replication/walsender_private.h"
***************
*** 63,68 **** static int compareWalFileNames(const void *a, const void *b);
--- 64,72 ----
/* Was the backup currently in-progress initiated in recovery mode? */
static bool backup_started_in_recovery = false;
+ /* Relative path of temporary statistics directory */
+ static char *statrelpath = NULL;
+
/*
* Size of each block sent into the tar stream for larger files.
*/
***************
*** 111,116 **** perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
--- 115,132 ----
&labelfile);
SendXlogRecPtrResult(startptr, starttli);
+ /*
+ * Calculate the relative path of temporary statistics directory
+ * in order to skip the files which are located in that directory later.
+ */
+ if (is_absolute_path(pgstat_stat_directory) &&
+ strncmp(pgstat_stat_directory, DataDir, datadirpathlen) == 0)
+ statrelpath = psprintf("./%s", pgstat_stat_directory + datadirpathlen + 1);
+ else if (strncmp(pgstat_stat_directory, "./", 2) != 0)
+ statrelpath = psprintf("./%s", pgstat_stat_directory);
+ else
+ statrelpath = pgstat_stat_directory;
+
PG_ENSURE_ERROR_CLEANUP(base_backup_cleanup, (Datum) 0);
{
List *tablespaces = NIL;
***************
*** 838,844 **** sendDir(char *path, int basepathlen, bool sizeonly, List *tablespaces)
sizeof(PG_AUTOCONF_FILENAME) + 4) == 0)
continue;
-
/*
* If there's a backup_label file, it belongs to a backup started by
* the user with pg_start_backup(). It is *not* correct for this
--- 854,859 ----
***************
*** 888,893 **** sendDir(char *path, int basepathlen, bool sizeonly, List *tablespaces)
--- 903,922 ----
}
/*
+ * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped
+ * even when stats_temp_directory is set because PGSS_TEXT_FILE is
+ * always created there.
+ */
+ if ((statrelpath != NULL && strcmp(pathbuf, statrelpath) == 0) ||
+ strncmp(de->d_name, PG_STAT_TMP_DIR, strlen(PG_STAT_TMP_DIR)) == 0)
+ {
+ if (!sizeonly)
+ _tarWriteHeader(pathbuf + basepathlen + 1, NULL, &statbuf);
+ size += 512;
+ continue;
+ }
+
+ /*
* We can skip pg_xlog, the WAL segments need to be fetched from the
* WAL archive anyway. But include it as an empty directory anyway, so
* we get permissions right.
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 3162,3168 **** static struct config_string ConfigureNamesString[] =
GUC_SUPERUSER_ONLY
},
&pgstat_temp_directory,
! "pg_stat_tmp",
check_canonical_path, assign_pgstat_temp_directory, NULL
},
--- 3162,3168 ----
GUC_SUPERUSER_ONLY
},
&pgstat_temp_directory,
! PG_STAT_TMP_DIR,
check_canonical_path, assign_pgstat_temp_directory, NULL
},
*** a/src/include/pgstat.h
--- b/src/include/pgstat.h
***************
*** 20,25 ****
--- 20,28 ----
#include "utils/relcache.h"
+ /* Default directory to store temporary statistics data in */
+ #define PG_STAT_TMP_DIR "pg_stat_tmp"
+
/* Values for track_functions GUC variable --- order is significant! */
typedef enum TrackFunctionsLevel
{
2014-01-31 Fujii Masao <masao.fujii@gmail.com>
On Tue, Jan 28, 2014 at 5:51 PM, Magnus Hagander <magnus@hagander.net>
wrote:On Tue, Jan 28, 2014 at 6:11 AM, Amit Kapila <amit.kapila16@gmail.com>
wrote:On Tue, Jan 28, 2014 at 9:26 AM, Fujii Masao <masao.fujii@gmail.com>
wrote:Hi,
The files in pg_stat_tmp directory don't need to be backed up because
they are
basically reset at the archive recovery. So I think it's worth
changing pg_basebackup
so that it skips any files in pg_stat_tmp directory. Thought?I think this is good idea, but can't it also avoid
PGSTAT_STAT_PERMANENT_TMPFILE along with temp files in
pg_stat_tmpAll stats files should be excluded. IIRC the
PGSTAT_STAT_PERMANENT_TMPFILE
refers to just the global one. You want to exclude based on
PGSTAT_STAT_PERMANENT_DIRECTORY (and of course based on the guc
stats_temp_directory if it's in PGDATA.Attached patch changes basebackup.c so that it skips all files in both
pg_stat_tmp
and stats_temp_directory. Even when a user sets stats_temp_directory
to the directory
other than pg_stat_tmp, we need to skip the files in pg_stat_tmp. Because,
per recent change of pg_stat_statements, the external query file is
always created there.
+1.
And, I'd like to also skip pg_log directory because security reason.
If you have time and get community agreed,
could you create these patch after committed your patch?
I don't want to bother you.
Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center
On Fri, Jan 31, 2014 at 10:18 PM, Mitsumasa KONDO
<kondo.mitsumasa@gmail.com> wrote:
2014-01-31 Fujii Masao <masao.fujii@gmail.com>
On Tue, Jan 28, 2014 at 5:51 PM, Magnus Hagander <magnus@hagander.net>
wrote:On Tue, Jan 28, 2014 at 6:11 AM, Amit Kapila <amit.kapila16@gmail.com>
wrote:On Tue, Jan 28, 2014 at 9:26 AM, Fujii Masao <masao.fujii@gmail.com>
wrote:Hi,
The files in pg_stat_tmp directory don't need to be backed up because
they are
basically reset at the archive recovery. So I think it's worth
changing pg_basebackup
so that it skips any files in pg_stat_tmp directory. Thought?I think this is good idea, but can't it also avoid
PGSTAT_STAT_PERMANENT_TMPFILE along with temp files in
pg_stat_tmpAll stats files should be excluded. IIRC the
PGSTAT_STAT_PERMANENT_TMPFILE
refers to just the global one. You want to exclude based on
PGSTAT_STAT_PERMANENT_DIRECTORY (and of course based on the guc
stats_temp_directory if it's in PGDATA.Attached patch changes basebackup.c so that it skips all files in both
pg_stat_tmp
and stats_temp_directory. Even when a user sets stats_temp_directory
to the directory
other than pg_stat_tmp, we need to skip the files in pg_stat_tmp. Because,
per recent change of pg_stat_statements, the external query file is
always created there.+1.
And, I'd like to also skip pg_log directory because security reason.
Yeah, I was thinking that, too. I'm not sure whether including log files
in backup really increases the security risk, though. There are already
very important data, i.e., database, in backups. Anyway, since
the amount of log files can be very large and they are not essential
for recovery, it's worth considering whether to exclude them. OTOH,
I'm sure that some users prefer current behavior for some reasons.
So I think that it's better to expose the pg_basebackup option
specifying whether log files are included in backups or not.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2014-01-31 Fujii Masao <masao.fujii@gmail.com>:
On Fri, Jan 31, 2014 at 10:18 PM, Mitsumasa KONDO
<kondo.mitsumasa@gmail.com> wrote:2014-01-31 Fujii Masao <masao.fujii@gmail.com>
On Tue, Jan 28, 2014 at 5:51 PM, Magnus Hagander <magnus@hagander.net>
wrote:On Tue, Jan 28, 2014 at 6:11 AM, Amit Kapila <amit.kapila16@gmail.com
wrote:
On Tue, Jan 28, 2014 at 9:26 AM, Fujii Masao <masao.fujii@gmail.com>
wrote:Hi,
The files in pg_stat_tmp directory don't need to be backed up
because
they are
basically reset at the archive recovery. So I think it's worth
changing pg_basebackup
so that it skips any files in pg_stat_tmp directory. Thought?I think this is good idea, but can't it also avoid
PGSTAT_STAT_PERMANENT_TMPFILE along with temp files in
pg_stat_tmpAll stats files should be excluded. IIRC the
PGSTAT_STAT_PERMANENT_TMPFILE
refers to just the global one. You want to exclude based on
PGSTAT_STAT_PERMANENT_DIRECTORY (and of course based on the guc
stats_temp_directory if it's in PGDATA.Attached patch changes basebackup.c so that it skips all files in both
pg_stat_tmp
and stats_temp_directory. Even when a user sets stats_temp_directory
to the directory
other than pg_stat_tmp, we need to skip the files in pg_stat_tmp.Because,
per recent change of pg_stat_statements, the external query file is
always created there.+1.
And, I'd like to also skip pg_log directory because security reason.
Yeah, I was thinking that, too. I'm not sure whether including log files
in backup really increases the security risk, though. There are already
very important data, i.e., database, in backups. Anyway, since
the amount of log files can be very large and they are not essential
for recovery, it's worth considering whether to exclude them. OTOH,
I'm sure that some users prefer current behavior for some reasons.
So I think that it's better to expose the pg_basebackup option
specifying whether log files are included in backups or not.
I'm with you. Thanks a lot !
Regards,
--
Mitsumsasa KONDO
NTT Open Source Software Center
On Fri, Jan 31, 2014 at 8:40 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
Yeah, I was thinking that, too. I'm not sure whether including log files
in backup really increases the security risk, though. There are already
very important data, i.e., database, in backups. Anyway, since
the amount of log files can be very large and they are not essential
for recovery, it's worth considering whether to exclude them. OTOH,
I'm sure that some users prefer current behavior for some reasons.
So I think that it's better to expose the pg_basebackup option
specifying whether log files are included in backups or not.
I don't really see how this can work reliably; pg_log isn't a
hard-coded name, but rather a GUC that users can leave set to that
value or set to any other value they choose.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Feb 1, 2014 at 2:08 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Jan 31, 2014 at 8:40 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
Yeah, I was thinking that, too. I'm not sure whether including log files
in backup really increases the security risk, though. There are already
very important data, i.e., database, in backups. Anyway, since
the amount of log files can be very large and they are not essential
for recovery, it's worth considering whether to exclude them. OTOH,
I'm sure that some users prefer current behavior for some reasons.
So I think that it's better to expose the pg_basebackup option
specifying whether log files are included in backups or not.I don't really see how this can work reliably; pg_log isn't a
hard-coded name, but rather a GUC that users can leave set to that
value or set to any other value they choose.
I'm thinking to change basebackup.c so that it compares the
name of the directory that it's trying to back up and the setting
value of log_directory parameter, then, if they are the same,
it just skips the directory. The patch that I sent upthread does
this regarding stats_temp_directory.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/2/14, 10:23 AM, Fujii Masao wrote:
I'm thinking to change basebackup.c so that it compares the
name of the directory that it's trying to back up and the setting
value of log_directory parameter, then, if they are the same,
it just skips the directory. The patch that I sent upthread does
this regarding stats_temp_directory.
I'm undecided on whether log files should be copied, but in case we
decide not to, it needs to be considered whether we at least recreate
the pg_log directory on the standby. Otherwise weird things will happen
when you start the standby, and it would introduce an extra fixup step
to sort that out.
Extra credit for doing something useful when pg_log is a symlink.
I fear, however, that if you end up implementing all that logic, it
would become too much special magic.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Feb 3, 2014 at 6:57 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 2/2/14, 10:23 AM, Fujii Masao wrote:
I'm thinking to change basebackup.c so that it compares the
name of the directory that it's trying to back up and the setting
value of log_directory parameter, then, if they are the same,
it just skips the directory. The patch that I sent upthread does
this regarding stats_temp_directory.I'm undecided on whether log files should be copied, but in case we
decide not to, it needs to be considered whether we at least recreate
the pg_log directory on the standby. Otherwise weird things will happen
when you start the standby, and it would introduce an extra fixup step
to sort that out.
Yes, basically we should skip only files under pg_log, but not pg_log
directory itself.
Extra credit for doing something useful when pg_log is a symlink.
I fear, however, that if you end up implementing all that logic, it
would become too much special magic.
ISTM that pg_xlog has already been handled in that way by basebackup.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Jan 31, 2014 at 9:29 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
On Tue, Jan 28, 2014 at 5:51 PM, Magnus Hagander <magnus@hagander.net> wrote:
On Tue, Jan 28, 2014 at 6:11 AM, Amit Kapila <amit.kapila16@gmail.com>
wrote:On Tue, Jan 28, 2014 at 9:26 AM, Fujii Masao <masao.fujii@gmail.com>
wrote:Hi,
The files in pg_stat_tmp directory don't need to be backed up because
they are
basically reset at the archive recovery. So I think it's worth
changing pg_basebackup
so that it skips any files in pg_stat_tmp directory. Thought?I think this is good idea, but can't it also avoid
PGSTAT_STAT_PERMANENT_TMPFILE along with temp files in
pg_stat_tmpAll stats files should be excluded. IIRC the PGSTAT_STAT_PERMANENT_TMPFILE
refers to just the global one. You want to exclude based on
PGSTAT_STAT_PERMANENT_DIRECTORY (and of course based on the guc
stats_temp_directory if it's in PGDATA.Attached patch changes basebackup.c so that it skips all files in both
pg_stat_tmp
and stats_temp_directory. Even when a user sets stats_temp_directory
to the directory
other than pg_stat_tmp, we need to skip the files in pg_stat_tmp. Because,
per recent change of pg_stat_statements, the external query file is
always created there.
Committed.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi.
This is a followup to a 2014-02 discussion that led to pg_stats_temp
being excluded from pg_basebackup. At the time, it was discussed to
exclude pg_log as well, but nothing eventually came of that.
Recently, one of our customers has had a basebackup fail because pg_log
contained files that were >8GB:
FATAL: archive member "pg_log/postgresql-20150119.log" too large for tar format
I think pg_basebackup should also skip pg_log entries, as it does for
pg_stats_temp and pg_replslot, etc. I've attached a patch along those
lines for discussion.
-- Abhijit
P.S. Aren't we leaking statrelpath?
Attachments:
0001-Skip-files-in-pg_log-during-basebackup.patchtext/x-diff; charset=us-asciiDownload
>From 8db162c1385b1cdd2b0e666975b76aa814f09f35 Mon Sep 17 00:00:00 2001
From: Abhijit Menon-Sen <ams@2ndQuadrant.com>
Date: Mon, 27 Apr 2015 12:58:52 +0530
Subject: Skip files in pg_log during basebackup
---
src/backend/replication/basebackup.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 1e86e4c..cc75a03 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -27,6 +27,7 @@
#include "nodes/pg_list.h"
#include "pgtar.h"
#include "pgstat.h"
+#include "postmaster/syslogger.h"
#include "replication/basebackup.h"
#include "replication/walsender.h"
#include "replication/walsender_private.h"
@@ -72,6 +73,9 @@ static bool backup_started_in_recovery = false;
/* Relative path of temporary statistics directory */
static char *statrelpath = NULL;
+/* Relative path to log directory */
+static char logpath[MAXPGPATH];
+
/*
* Size of each block sent into the tar stream for larger files.
*/
@@ -157,6 +161,18 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
else
statrelpath = pgstat_stat_directory;
+ /*
+ * Do the same for the log_directory.
+ */
+
+ if (is_absolute_path(Log_directory) &&
+ path_is_prefix_of_path(DataDir, Log_directory))
+ snprintf(logpath, MAXPGPATH, "./%s", Log_directory + datadirpathlen + 1);
+ else if (strncmp(Log_directory, "./", 2) != 0)
+ snprintf(logpath, MAXPGPATH, "./%s", Log_directory);
+ else
+ strncpy(logpath, Log_directory, MAXPGPATH);
+
/* Add a node for the base directory at the end */
ti = palloc0(sizeof(tablespaceinfo));
ti->size = opt->progress ? sendDir(".", 1, true, tablespaces, true) : -1;
@@ -965,6 +981,19 @@ sendDir(char *path, int basepathlen, bool sizeonly, List *tablespaces,
}
/*
+ * We can skip pg_log (or whatever log_directory is set to, if
+ * it's under the data directory), but include it as an empty
+ * directory anyway, so we get permissions right.
+ */
+ if (strcmp(pathbuf, logpath) == 0)
+ {
+ if (!sizeonly)
+ _tarWriteHeader(pathbuf + basepathlen + 1, NULL, &statbuf);
+ size += 512;
+ continue;
+ }
+
+ /*
* Skip pg_replslot, not useful to copy. But include it as an empty
* directory anyway, so we get permissions right.
*/
--
1.9.1
On Mon, Jun 8, 2015 at 12:42 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
This is a followup to a 2014-02 discussion that led to pg_stats_temp
being excluded from pg_basebackup. At the time, it was discussed to
exclude pg_log as well, but nothing eventually came of that.
It seems to be that:
/messages/by-id/CAHGQGwH0OKZ6cKpJKCWOjGa3ejwfFm1eNrmRO3dkdoTeaai-eg@mail.gmail.com
Recently, one of our customers has had a basebackup fail because pg_log
contained files that were >8GB:
FATAL: archive member "pg_log/postgresql-20150119.log" too large for tar formatI think pg_basebackup should also skip pg_log entries, as it does for
pg_stats_temp and pg_replslot, etc. I've attached a patch along those
lines for discussion.
And a recent discussion about that is this one:
/messages/by-id/82897A1301080E4B8E461DDAA0FFCF142A1B2660@SYD1216
Bringing the point: some users may want to keep log files in a base
backup, and some users may want to skip some of them, and not only
pg_log. Hence we may want more flexibility than what is proposed here.
Regards,
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
At 2015-06-08 13:09:02 +0900, michael.paquier@gmail.com wrote:
It seems to be that:
/messages/by-id/CAHGQGwH0OKZ6cKpJKCWOjGa3ejwfFm1eNrmRO3dkdoTeaai-eg@mail.gmail.com
(Note that this is about calculating the wrong size, whereas my bug is
about the file being too large to be written to a tar archive.)
And a recent discussion about that is this one:
/messages/by-id/82897A1301080E4B8E461DDAA0FFCF142A1B2660@SYD1216
Oh, sorry, I somehow did miss that thread entirely. Thanks for the
pointer. (I've added Vaishnavi to the Cc: list here.)
I'm not convinced that we need a mechanism to let people exclude the
torrent files they've stored in their data directory, but if we have to
do it, the idea of having a GUC setting rather than specifying excludes
on the basebackup command line each time does have a certain appeal.
Anyone else interested in doing it that way?
-- Abhijit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Jun 8, 2015 at 12:09 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
Recently, one of our customers has had a basebackup fail because pg_log
contained files that were >8GB:
FATAL: archive member "pg_log/postgresql-20150119.log" too large for tar formatI think pg_basebackup should also skip pg_log entries, as it does for
pg_stats_temp and pg_replslot, etc. I've attached a patch along those
lines for discussion.And a recent discussion about that is this one:
/messages/by-id/82897A1301080E4B8E461DDAA0FFCF142A1B2660@SYD1216
Bringing the point: some users may want to keep log files in a base
backup, and some users may want to skip some of them, and not only
pg_log. Hence we may want more flexibility than what is proposed here.
That seems pretty thin. If you're taking a base backup, your goal is
to create a standby. Copying logs is in no way an integral part of
that, and we would not copy them if they were stored outside the data
directory. If we accept the proposal that this needs to be more
complicated, will we also accept a proposal to make pg_basebackup
include relevant files from /var/log when the PostgreSQL logs are
stored there?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jun 10, 2015 at 8:29 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Jun 8, 2015 at 12:09 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:Recently, one of our customers has had a basebackup fail because pg_log
contained files that were >8GB:
FATAL: archive member "pg_log/postgresql-20150119.log" too large fortar format
I think pg_basebackup should also skip pg_log entries, as it does for
pg_stats_temp and pg_replslot, etc. I've attached a patch along those
lines for discussion.And a recent discussion about that is this one:
/messages/by-id/82897A1301080E4B8E461DDAA0FFCF142A1B2660@SYD1216
Bringing the point: some users may want to keep log files in a base
backup, and some users may want to skip some of them, and not only
pg_log. Hence we may want more flexibility than what is proposed here.That seems pretty thin. If you're taking a base backup, your goal is
to create a standby.
Mine goal isn't that. My goal is to have a consistent backup without
having to shut down the server to take a cold one, or having to manually
juggle the pg_start_backup, etc. commands. I do occasionally use it start
up a standby for training/testing purposes, but mostly it is for D-R (in
which I would rather have the logs) and for cloning test/dev/QA
environments (in which case I go delete the logs if I don't want them)
Copying logs is in no way an integral part of
that, and we would not copy them if they were stored outside the data
directory. If we accept the proposal that this needs to be more
complicated, will we also accept a proposal to make pg_basebackup
include relevant files from /var/log when the PostgreSQL logs are
stored there?
I think it is pretty intuitive that if you have your logs go to pg_log,
they get backed up with the other pg_ stuff, and if you change it go
elsewhere, then you need to handle it yourself.
Cheers,
Jeff
On 2015-06-10 09:57:17 -0700, Jeff Janes wrote:
Mine goal isn't that. My goal is to have a consistent backup without
having to shut down the server to take a cold one, or having to manually
juggle the pg_start_backup, etc. commands.
A basebackup won't necessarily give you a consistent log though...
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 06/10/2015 10:01 AM, Andres Freund wrote:
On 2015-06-10 09:57:17 -0700, Jeff Janes wrote:
Mine goal isn't that. My goal is to have a consistent backup without
having to shut down the server to take a cold one, or having to manually
juggle the pg_start_backup, etc. commands.A basebackup won't necessarily give you a consistent log though...
I am -1 on this idea. It just doesn't seem to make sense. There are too
many variables where it won't work or won't be relevant.
JD
--
Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jun 10, 2015 at 1:12 PM, Joshua D. Drake <jd@commandprompt.com> wrote:
On 06/10/2015 10:01 AM, Andres Freund wrote:
On 2015-06-10 09:57:17 -0700, Jeff Janes wrote:
Mine goal isn't that. My goal is to have a consistent backup without
having to shut down the server to take a cold one, or having to manually
juggle the pg_start_backup, etc. commands.A basebackup won't necessarily give you a consistent log though...
I am -1 on this idea. It just doesn't seem to make sense. There are too many
variables where it won't work or won't be relevant.
I'm not clear on which of these options you are voting for:
(1) include pg_log in pg_basebackup as we do currently
(2) exclude it
(3) add a switch controlling whether or not it gets excluded
I can live with (3), but I bet most people want (2).
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 06/10/2015 10:22 AM, Robert Haas wrote:
On Wed, Jun 10, 2015 at 1:12 PM, Joshua D. Drake <jd@commandprompt.com> wrote:
On 06/10/2015 10:01 AM, Andres Freund wrote:
On 2015-06-10 09:57:17 -0700, Jeff Janes wrote:
Mine goal isn't that. My goal is to have a consistent backup without
having to shut down the server to take a cold one, or having to manually
juggle the pg_start_backup, etc. commands.A basebackup won't necessarily give you a consistent log though...
I am -1 on this idea. It just doesn't seem to make sense. There are too many
variables where it won't work or won't be relevant.I'm not clear on which of these options you are voting for:
(1) include pg_log in pg_basebackup as we do currently
(2) exclude it
(3) add a switch controlling whether or not it gets excludedI can live with (3), but I bet most people want (2).
Sorry I wasn't clear. #2
Sincerely,
JD
--
Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
At 2015-06-10 13:22:27 -0400, robertmhaas@gmail.com wrote:
I'm not clear on which of these options you are voting for:
(1) include pg_log in pg_basebackup as we do currently
(2) exclude it
(3) add a switch controlling whether or not it gets excludedI can live with (3), but I bet most people want (2).
Thanks for spelling out the options.
I strongly prefer (2), but I could live with (3) if it were done as a
GUC setting. (And if that's what we decide to do, I'm willing to write
up the patch.)
Whether or not it's a good idea to let one's logfiles grow to >8GB, the
fact that doing so breaks base backups means that being able to exclude
pg_log *somehow* is more of a necessity than personal preference.
On the other hand, I don't like the idea of doing (3) by adding command
line arguments to pg_basebackup and adding a new option to the command.
I don't think that level of "flexibility" is justified; it would also
make it easier to end up with a broken base backup (by inadvertently
excluding more than you meant to).
-- Abhijit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jun 11, 2015 at 2:20 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
At 2015-06-10 13:22:27 -0400, robertmhaas@gmail.com wrote:
I'm not clear on which of these options you are voting for:
(1) include pg_log in pg_basebackup as we do currently
(2) exclude it
(3) add a switch controlling whether or not it gets excludedI can live with (3), but I bet most people want (2).
Thanks for spelling out the options.
I strongly prefer (2), but I could live with (3) if it were done as a
GUC setting. (And if that's what we decide to do, I'm willing to write
up the patch.)Whether or not it's a good idea to let one's logfiles grow to >8GB, the
fact that doing so breaks base backups means that being able to exclude
pg_log *somehow* is more of a necessity than personal preference.On the other hand, I don't like the idea of doing (3) by adding command
line arguments to pg_basebackup and adding a new option to the command.
I don't think that level of "flexibility" is justified; it would also
make it easier to end up with a broken base backup (by inadvertently
excluding more than you meant to).
After spending the night thinking about that, honestly, I think that
we should go with (2) and keep the base backup as light-weight as
possible and not bother about a GUC. (3) would need some extra
intelligence to decide if some files can be skipped or not. Imagine
for example --skip-files=global/pg_control or --skip-files=pg_clog
(because it *is* a log file with much data), that would just corrupt
silently your backup, but I guess that it is what you had in mind. In
any case (3) is not worth the maintenance burden because we would need
to update the things to filter each time a new important folder is
added in PGDATA by a patch.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2015-06-11 PM 02:20, Abhijit Menon-Sen wrote:
At 2015-06-10 13:22:27 -0400, robertmhaas@gmail.com wrote:
(1) include pg_log in pg_basebackup as we do currently
(2) exclude it
(3) add a switch controlling whether or not it gets excludedI can live with (3), but I bet most people want (2).
Thanks for spelling out the options.
I strongly prefer (2), but I could live with (3) if it were done as a
GUC setting. (And if that's what we decide to do, I'm willing to write
up the patch.)Whether or not it's a good idea to let one's logfiles grow to >8GB, the
fact that doing so breaks base backups means that being able to exclude
pg_log *somehow* is more of a necessity than personal preference.On the other hand, I don't like the idea of doing (3) by adding command
line arguments to pg_basebackup and adding a new option to the command.
I don't think that level of "flexibility" is justified; it would also
make it easier to end up with a broken base backup (by inadvertently
excluding more than you meant to).
Maybe a combination of (2) and part of (3). In absence of any command line
argument, the behavior is (2), to exclude. Provide an option to *include* it
(-S/--serverlog).
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
At 2015-06-11 14:28:36 +0900, michael.paquier@gmail.com wrote:
After spending the night thinking about that, honestly, I think that
we should go with (2) and keep the base backup as light-weight as
possible and not bother about a GUC.
OK. Then the patch I posted earlier should be sufficient.
-- Abhijit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jun 11, 2015 at 2:39 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
At 2015-06-11 14:28:36 +0900, michael.paquier@gmail.com wrote:
After spending the night thinking about that, honestly, I think that
we should go with (2) and keep the base backup as light-weight as
possible and not bother about a GUC.OK. Then the patch I posted earlier should be sufficient.
Btw, one thing that 010_pg_basebackup.pl does not check is actually if
the files filtered by basebackup.c are included or not in the base
backup. We may want to add some extra checks regarding that...
Especially with your patch that filters things depending on if
log_directory is an absolute path or not.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Jun 11, 2015 7:38 AM, "Amit Langote" <Langote_Amit_f8@lab.ntt.co.jp>
wrote:
On 2015-06-11 PM 02:20, Abhijit Menon-Sen wrote:
At 2015-06-10 13:22:27 -0400, robertmhaas@gmail.com wrote:
(1) include pg_log in pg_basebackup as we do currently
(2) exclude it
(3) add a switch controlling whether or not it gets excludedI can live with (3), but I bet most people want (2).
Thanks for spelling out the options.
I strongly prefer (2), but I could live with (3) if it were done as a
GUC setting. (And if that's what we decide to do, I'm willing to write
up the patch.)Whether or not it's a good idea to let one's logfiles grow to >8GB, the
fact that doing so breaks base backups means that being able to exclude
pg_log *somehow* is more of a necessity than personal preference.On the other hand, I don't like the idea of doing (3) by adding command
line arguments to pg_basebackup and adding a new option to the command.
I don't think that level of "flexibility" is justified; it would also
make it easier to end up with a broken base backup (by inadvertently
excluding more than you meant to).Maybe a combination of (2) and part of (3). In absence of any command line
argument, the behavior is (2), to exclude. Provide an option to *include*
it
(-S/--serverlog)
I think it's useful enough to have a switch, but no problem to exclude it
by default. So I can definitely go for Amits suggestions.
I also don't feel strongly enough about it to put up any kind of fight if
the majority wants different :-)
/Magnus
Michael Paquier wrote:
After spending the night thinking about that, honestly, I think that
we should go with (2) and keep the base backup as light-weight as
possible and not bother about a GUC. (3) would need some extra
intelligence to decide if some files can be skipped or not. Imagine
for example --skip-files=global/pg_control or --skip-files=pg_clog
(because it *is* a log file with much data), that would just corrupt
silently your backup, but I guess that it is what you had in mind. In
any case (3) is not worth the maintenance burden because we would need
to update the things to filter each time a new important folder is
added in PGDATA by a patch.
If somebody sets log_directory=pg_clog/ they are screwed pretty badly,
aren't they. (I guess this is just a case of "don't do that").
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
At 2015-06-11 14:38:03 +0900, Langote_Amit_f8@lab.ntt.co.jp wrote:
On the other hand, I don't like the idea of doing (3) by adding
command line arguments to pg_basebackup and adding a new option to
the command. I don't think that level of "flexibility" is justified;
it would also make it easier to end up with a broken base backup (by
inadvertently excluding more than you meant to).Maybe a combination of (2) and part of (3). In absence of any command
line argument, the behavior is (2), to exclude. Provide an option to
*include* it (-S/--serverlog).
I don't like that idea any more than having the command-line argument to
exclude pg_log. (And people who store torrented files in PGDATA may like
it even less.)
-- Abhijit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers