Re: PATCH: Exclude temp relations from base backup
Thank you, pushed David Steele wrote: On 3/26/18 1:06 PM, Stephen Frost wrote: * Teodor Sigaev (teo...@sigaev.ru) wrote: Will autovacuum (or something else) complain about absense of relfile during orphan table deleting? I mean, you get a base backup without temp tables, then you try to run postgres on it and will it complain about existing record in pg_class and absence of corresponding relfile? I would certainly hope not considering that's what happens during regular crash recovery also, so if there's an issue with that, we'd have a problem in released versions. Agreed. The logic for pg_basebackup was modeled off RemovePgTempFiles() which is called at postmaster start. We are just doing the cleanup in advance (in the backup only, of course). Thanks, -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/
Re: PATCH: Exclude temp relations from base backup
On 3/26/18 1:06 PM, Stephen Frost wrote: > > * Teodor Sigaev (teo...@sigaev.ru) wrote: >> Will autovacuum (or something else) complain about absense of relfile during >> orphan table deleting? I mean, you get a base backup without temp tables, >> then you try to run postgres on it and will it complain about existing >> record in pg_class and absence of corresponding relfile? > > I would certainly hope not considering that's what happens during > regular crash recovery also, so if there's an issue with that, we'd have > a problem in released versions. Agreed. The logic for pg_basebackup was modeled off RemovePgTempFiles() which is called at postmaster start. We are just doing the cleanup in advance (in the backup only, of course). Thanks, -- -David da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: PATCH: Exclude temp relations from base backup
Greetings, * Teodor Sigaev (teo...@sigaev.ru) wrote: > Will autovacuum (or something else) complain about absense of relfile during > orphan table deleting? I mean, you get a base backup without temp tables, > then you try to run postgres on it and will it complain about existing > record in pg_class and absence of corresponding relfile? I would certainly hope not considering that's what happens during regular crash recovery also, so if there's an issue with that, we'd have a problem in released versions. There's an independent discussion that was being had recently about how to make sure those records in pg_class get cleaned up in a reasonable timeframe and don't lead to problems with wrap-arounds, but that's a different and pre-existing issue. Thanks! Stephen signature.asc Description: PGP signature
Re: PATCH: Exclude temp relations from base backup
Hi! Will autovacuum (or something else) complain about absense of relfile during orphan table deleting? I mean, you get a base backup without temp tables, then you try to run postgres on it and will it complain about existing record in pg_class and absence of corresponding relfile? David Steele wrote: On 3/13/18 12:34 PM, David Steele wrote: Updated the patch to change die() to BAIL_OUT() and use append_to_file() as suggested for another test patch. Updated patch now that the unlogged table exclusions have been committed [1]. Thanks, -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/
Re: PATCH: Exclude temp relations from base backup
On 3/13/18 12:34 PM, David Steele wrote:
> Updated the patch to change die() to BAIL_OUT() and use append_to_file()
> as suggested for another test patch.
Updated patch now that the unlogged table exclusions have been committed
[1].
Thanks,
--
-David
da...@pgmasters.net
[1]
https://www.postgresql.org/message-id/4d9be1c0-5c58-d9a0-7152-2771224910ae%40sigaev.ru
From e4fc8ff2b41091c0e266ecde8f91471adca26f9c Mon Sep 17 00:00:00 2001
From: David Steele
Date: Fri, 23 Mar 2018 12:39:59 -0400
Subject: [PATCH 1/1] Exclude temporary relations from base backup.
Exclude temporary relations during a base backup using the existing
looks_like_temp_rel_name() function for identification.
---
doc/src/sgml/protocol.sgml | 2 +-
src/backend/replication/basebackup.c | 10 ++
src/backend/storage/file/fd.c| 3 +-
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 49 +++-
src/include/storage/fd.h | 1 +
5 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 472bd3ef69..8c488506fa 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2565,7 +2565,7 @@ The commands accepted in replication mode are:
Various temporary files and directories created during the operation
of the PostgreSQL server, such as any file or directory beginning
- with pgsql_tmp.
+ with pgsql_tmp and temporary relations.
diff --git a/src/backend/replication/basebackup.c
b/src/backend/replication/basebackup.c
index eb6eb7206d..189e870b9d 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -1071,6 +1071,16 @@ sendDir(const char *path, int basepathlen, bool
sizeonly, List *tablespaces,
}
}
+ /* Exclude temporary relations */
+ if (isDbDir && looks_like_temp_rel_name(de->d_name))
+ {
+ elog(DEBUG2,
+"temporary relation file \"%s\" excluded from
backup",
+de->d_name);
+
+ continue;
+ }
+
snprintf(pathbuf, sizeof(pathbuf), "%s/%s", path, de->d_name);
/* Skip pg_control here to back up it last */
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 2a18e94ff4..d30a725f90 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -325,7 +325,6 @@ static void RemovePgTempFilesInDir(const char *tmpdirname,
bool missing_ok,
bool unlink_all);
static void RemovePgTempRelationFiles(const char *tsdirname);
static void RemovePgTempRelationFilesInDbspace(const char *dbspacedirname);
-static bool looks_like_temp_rel_name(const char *name);
static void walkdir(const char *path,
void (*action) (const char *fname, bool isdir, int elevel),
@@ -3192,7 +3191,7 @@ RemovePgTempRelationFilesInDbspace(const char
*dbspacedirname)
}
/* t_, or t__ */
-static bool
+bool
looks_like_temp_rel_name(const char *name)
{
int pos;
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 455c7fca0d..e6018de054 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -2,9 +2,10 @@ use strict;
use warnings;
use Cwd;
use Config;
+use File::Basename qw(basename dirname);
use PostgresNode;
use TestLib;
-use Test::More tests => 87;
+use Test::More tests => 93;
program_help_ok('pg_basebackup');
program_version_ok('pg_basebackup');
@@ -76,6 +77,18 @@ my $baseUnloggedPath = $node->safe_psql('postgres',
ok(-f "$pgdata/${baseUnloggedPath}_init", 'unlogged init fork in base');
ok(-f "$pgdata/$baseUnloggedPath", 'unlogged main fork in base');
+# Create files that look like temporary relations to ensure they are ignored.
+my $postgresOid = $node->safe_psql('postgres',
+ q{select oid from pg_database where datname = 'postgres'});
+
+my @tempRelationFiles = qw(t999_999 t_999.1 t999__vm
t9_9_vm.1);
+
+foreach my $filename (@tempRelationFiles)
+{
+ append_to_file("$pgdata/base/$postgresOid/$filename", 'TEMP_RELATION');
+}
+
+# Run base backup.
$node->command_ok([ 'pg_basebackup', '-D', "$tempdir/backup", '-X', 'none' ],
'pg_basebackup runs');
ok(-f "$tempdir/backup/PG_VERSION", 'backup was created');
@@ -112,6 +125,13 @@ ok(-f "$tempdir/backup/${baseUnloggedPath}_init",
ok(!-f "$tempdir/backup/$baseUnloggedPath",
'unlogged main fork not in backup');
+# Temp relations should not be copied.
+foreach my $filename (@tempRelationFiles)
+{
+ ok(!-f "$tempdir/backup/base/$postgresOid/$filename",
+ "base/$postgresOid/$filename not copied");
+}
+
# Mak
Re: PATCH: Exclude temp relations from base backup
Hi,
On 2/28/18 10:55 AM, David Steele wrote:
> This is a follow-up patch from the exclude unlogged relations discussion
> [1].
>
> The patch excludes temporary relations during a base backup using the
> existing looks_like_temp_rel_name() function for identification.
>
> It shares code to identify database directories from [1], so for now
> that has been duplicated in this patch to make it independent. I'll
> rebase depending on what gets committed first.
Updated the patch to change die() to BAIL_OUT() and use append_to_file()
as suggested for another test patch [1].
Regards,
--
-David
da...@pgmasters.net
[1]
https://www.postgresql.org/message-id/6bc5d931-5b00-279f-f65a-26e32de400a6%40pgmasters.net
From c83f3ae09bacb961c511ebe9cb1f9f79bf1e3d29 Mon Sep 17 00:00:00 2001
From: David Steele
Date: Tue, 13 Mar 2018 12:22:24 -0400
Subject: [PATCH 1/1] Exclude temporary relations from base backup.
Exclude temporary relations during a base backup using the existing
looks_like_temp_rel_name() function for identification.
It shares code to identify database directories with [1], so for now that has
been duplicated in this patch to make it independent. I'll rebase depending on
what gets committed first.
[1]
https://www.postgresql.org/message-id/04791bab-cb04-ba43-e9c0-664a4c1ffb2c%40pgmasters.net
---
doc/src/sgml/protocol.sgml | 2 +-
src/backend/replication/basebackup.c | 41 +++
src/backend/storage/file/fd.c| 3 +-
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 49 +++-
src/include/storage/fd.h | 1 +
5 files changed, 92 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 4fd61d7c2d..629a462574 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2565,7 +2565,7 @@ The commands accepted in replication mode are:
Various temporary files and directories created during the operation
of the PostgreSQL server, such as any file or directory beginning
- with pgsql_tmp.
+ with pgsql_tmp and temporary relations.
diff --git a/src/backend/replication/basebackup.c
b/src/backend/replication/basebackup.c
index 185f32a5f9..f0c3d13b2b 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -26,6 +26,7 @@
#include "nodes/pg_list.h"
#include "pgtar.h"
#include "pgstat.h"
+#include "port.h"
#include "postmaster/syslogger.h"
#include "replication/basebackup.h"
#include "replication/walsender.h"
@@ -958,6 +959,36 @@ sendDir(const char *path, int basepathlen, bool sizeonly,
List *tablespaces,
charpathbuf[MAXPGPATH * 2];
struct stat statbuf;
int64 size = 0;
+ const char *lastDir; /* Split last dir from
parent path. */
+ boolisDbDir = false;/* Does this directory contain
relations? */
+
+ /*
+* Determine if the current path is a database directory that can
+* contain relations.
+*
+* Start by finding the location of the delimiter between the parent
+* path and the current path.
+*/
+ lastDir = last_dir_separator(path);
+
+ /* Does this path look like a database path (i.e. all digits)? */
+ if (lastDir != NULL &&
+ strspn(lastDir + 1, "0123456789") == strlen(lastDir + 1))
+ {
+ /* Part of path that contains the parent directory. */
+ int parentPathLen = lastDir - path;
+
+ /*
+* Mark path as a database directory if the parent path is
either
+* $PGDATA/base or a tablespace version path.
+*/
+ if (strncmp(path, "./base", parentPathLen) == 0 ||
+ (parentPathLen >= (sizeof(TABLESPACE_VERSION_DIRECTORY)
- 1) &&
+strncmp(lastDir -
(sizeof(TABLESPACE_VERSION_DIRECTORY) - 1),
+TABLESPACE_VERSION_DIRECTORY,
+sizeof(TABLESPACE_VERSION_DIRECTORY) -
1) == 0))
+ isDbDir = true;
+ }
dir = AllocateDir(path);
while ((de = ReadDir(dir, path)) != NULL)
@@ -1007,6 +1038,16 @@ sendDir(const char *path, int basepathlen, bool
sizeonly, List *tablespaces,
if (excludeFound)
continue;
+ /* Exclude temporary relations */
+ if (isDbDir && looks_like_temp_rel_name(de->d_name))
+ {
+ elog(DEBUG2,
+"temporary relation file \"%s\" excluded from
backup",
+de->d_name);
+
+ continue;
+ }
+
snprintf(pathbuf, sizeof(pathbuf), "%s/%s", path, de->d_name);
