PATCH: Exclude temp relations from base backup

Started by David Steelealmost 8 years ago7 messages
#1David Steele
david@pgmasters.net
1 attachment(s)

This is a follow-up patch from the exclude unlogged relations discussion
[1]: /messages/by-id/04791bab-cb04-ba43-e9c0-664a4c1ffb2c@pgmasters.net

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]/messages/by-id/04791bab-cb04-ba43-e9c0-664a4c1ffb2c@pgmasters.net, so for now
that has been duplicated in this patch to make it independent. I'll
rebase depending on what gets committed first.

Thanks,
--
-David
david@pgmasters.net

[1]: /messages/by-id/04791bab-cb04-ba43-e9c0-664a4c1ffb2c@pgmasters.net
/messages/by-id/04791bab-cb04-ba43-e9c0-664a4c1ffb2c@pgmasters.net

Attachments:

backup-exclude-temp-rel-v1.patchtext/plain; charset=UTF-8; name=backup-exclude-temp-rel-v1.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 3cec9e0b0c..3880a9134b 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2549,7 +2549,7 @@ The commands accepted in walsender mode are:
         <para>
          Various temporary files and directories created during the operation
          of the PostgreSQL server, such as any file or directory beginning
-         with <filename>pgsql_tmp</filename>.
+         with <filename>pgsql_tmp</filename> and temporary relations.
         </para>
        </listitem>
        <listitem>
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,
 	char		pathbuf[MAXPGPATH * 2];
 	struct stat statbuf;
 	int64		size = 0;
+	const char 	*lastDir;			/* Split last dir from parent path. */
+	bool 		isDbDir = 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);
 
 		/* 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<digits>_<digits>, or t<digits>_<digits>_<forkname> */
-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 cdf4f5be37..16bd91f63e 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 => 79;
+use Test::More tests => 85;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -66,6 +67,20 @@ foreach my $filename (
 # positive.
 $node->safe_psql('postgres', 'SELECT 1;');
 
+# 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 t9999_999.1 t999_9999_vm t99999_99999_vm.1);
+
+foreach my $filename (@tempRelationFiles)
+{
+	open my $file, '>>', "$pgdata/base/$postgresOid/$filename";
+	print $file "TEMPRELATION";
+	close $file;
+}
+
+# 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');
@@ -96,6 +111,13 @@ foreach my $filename (
 	ok(!-f "$tempdir/backup/$filename", "$filename not copied");
 }
 
+# Temp relations should not be copied.
+foreach my $filename (@tempRelationFiles)
+{
+	ok(!-f "$tempdir/backup/base/$postgresOid/$filename",
+	   "base/$postgresOid/$filename not copied");
+}
+
 # Make sure existing backup_label was ignored.
 isnt(slurp_file("$tempdir/backup/backup_label"),
 	'DONOTCOPY', 'existing backup_label not copied');
@@ -177,6 +199,24 @@ SKIP:
 	my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
 	is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
 
+	# Create files that look like temporary relations to ensure they are ignored
+	# in a tablespace.
+	my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
+	my $tblSpc1Id = basename(dirname(dirname($node->safe_psql('postgres',
+		q{select pg_relation_filepath('test1')}))));
+
+	foreach my $filename (@tempRelationFiles)
+	{
+		my $filepath =
+			"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
+
+		open(my $file, '>>', $filepath)
+			or die "unable to open $filepath";
+		print($file, "TEMPRELATION")
+			or die "unable to write $filepath";
+		close $file;
+	}
+
 	$node->command_fails(
 		[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
 		'plain format with tablespaces fails without tablespace mapping');
@@ -195,6 +235,20 @@ SKIP:
 		"tablespace symlink was updated");
 	closedir $dh;
 
+	# Temp relations should not be copied.
+	foreach my $filename (@tempRelationFiles)
+	{
+		ok(!-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+		   "[tblspc1]/$postgresOid/$filename not copied");
+
+		# Also remove temp relation files or tablespace drop will fail.
+		my $filepath =
+			"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
+
+		unlink($filepath)
+			or die "unable to unlink $filepath";
+	}
+
 	ok( -d "$tempdir/backup1/pg_replslot",
 		'pg_replslot symlink copied as directory');
 
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index 4244e7b1fd..e49b42ce86 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -124,6 +124,7 @@ extern void AtEOXact_Files(void);
 extern void AtEOSubXact_Files(bool isCommit, SubTransactionId mySubid,
 				  SubTransactionId parentSubid);
 extern void RemovePgTempFiles(void);
+extern bool looks_like_temp_rel_name(const char *name);
 
 extern int	pg_fsync(int fd);
 extern int	pg_fsync_no_writethrough(int fd);
#2David Steele
david@pgmasters.net
In reply to: David Steele (#1)
1 attachment(s)
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]/messages/by-id/6bc5d931-5b00-279f-f65a-26e32de400a6@pgmasters.net.

Regards,
--
-David
david@pgmasters.net

[1]: /messages/by-id/6bc5d931-5b00-279f-f65a-26e32de400a6@pgmasters.net
/messages/by-id/6bc5d931-5b00-279f-f65a-26e32de400a6@pgmasters.net

Attachments:

backup-exclude-temp-rel-v2.patchtext/plain; charset=UTF-8; name=backup-exclude-temp-rel-v2.patch; x-mac-creator=0; x-mac-type=0Download
From c83f3ae09bacb961c511ebe9cb1f9f79bf1e3d29 Mon Sep 17 00:00:00 2001
From: David Steele <david@pgmasters.net>
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:
         <para>
          Various temporary files and directories created during the operation
          of the PostgreSQL server, such as any file or directory beginning
-         with <filename>pgsql_tmp</filename>.
+         with <filename>pgsql_tmp</filename> and temporary relations.
         </para>
        </listitem>
        <listitem>
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,
 	char		pathbuf[MAXPGPATH * 2];
 	struct stat statbuf;
 	int64		size = 0;
+	const char 	*lastDir;			/* Split last dir from parent path. */
+	bool 		isDbDir = 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);
 
 		/* 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<digits>_<digits>, or t<digits>_<digits>_<forkname> */
-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 cdf4f5be37..096472fd0d 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 => 79;
+use Test::More tests => 85;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -66,6 +67,18 @@ foreach my $filename (
 # positive.
 $node->safe_psql('postgres', 'SELECT 1;');
 
+# 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 t9999_999.1 t999_9999_vm t99999_99999_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');
@@ -96,6 +109,13 @@ foreach my $filename (
 	ok(!-f "$tempdir/backup/$filename", "$filename not copied");
 }
 
+# Temp relations should not be copied.
+foreach my $filename (@tempRelationFiles)
+{
+	ok(!-f "$tempdir/backup/base/$postgresOid/$filename",
+	   "base/$postgresOid/$filename not copied");
+}
+
 # Make sure existing backup_label was ignored.
 isnt(slurp_file("$tempdir/backup/backup_label"),
 	'DONOTCOPY', 'existing backup_label not copied');
@@ -177,6 +197,19 @@ SKIP:
 	my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
 	is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
 
+	# Create files that look like temporary relations to ensure they are ignored
+	# in a tablespace.
+	my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
+	my $tblSpc1Id = basename(dirname(dirname($node->safe_psql('postgres',
+		q{select pg_relation_filepath('test1')}))));
+
+	foreach my $filename (@tempRelationFiles)
+	{
+		append_to_file(
+			"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+			'TEMP_RELATION');
+	}
+
 	$node->command_fails(
 		[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
 		'plain format with tablespaces fails without tablespace mapping');
@@ -195,6 +228,20 @@ SKIP:
 		"tablespace symlink was updated");
 	closedir $dh;
 
+	# Temp relations should not be copied.
+	foreach my $filename (@tempRelationFiles)
+	{
+		ok(!-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+		   "[tblspc1]/$postgresOid/$filename not copied");
+
+		# Also remove temp relation files or tablespace drop will fail.
+		my $filepath =
+			"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
+
+		unlink($filepath)
+			or BAIL_OUT("unable to unlink $filepath");
+	}
+
 	ok( -d "$tempdir/backup1/pg_replslot",
 		'pg_replslot symlink copied as directory');
 
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index 4244e7b1fd..e49b42ce86 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -124,6 +124,7 @@ extern void AtEOXact_Files(void);
 extern void AtEOSubXact_Files(bool isCommit, SubTransactionId mySubid,
 				  SubTransactionId parentSubid);
 extern void RemovePgTempFiles(void);
+extern bool looks_like_temp_rel_name(const char *name);
 
 extern int	pg_fsync(int fd);
 extern int	pg_fsync_no_writethrough(int fd);
-- 
2.14.3 (Apple Git-98)

#3David Steele
david@pgmasters.net
In reply to: David Steele (#2)
1 attachment(s)
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]: /messages/by-id/4d9be1c0-5c58-d9a0-7152-2771224910ae@sigaev.ru

Thanks,
--
-David
david@pgmasters.net

[1]: /messages/by-id/4d9be1c0-5c58-d9a0-7152-2771224910ae@sigaev.ru
/messages/by-id/4d9be1c0-5c58-d9a0-7152-2771224910ae@sigaev.ru

Attachments:

backup-exclude-temp-rel-v3.patchtext/plain; charset=UTF-8; name=backup-exclude-temp-rel-v3.patch; x-mac-creator=0; x-mac-type=0Download
From e4fc8ff2b41091c0e266ecde8f91471adca26f9c Mon Sep 17 00:00:00 2001
From: David Steele <david@pgmasters.net>
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:
         <para>
          Various temporary files and directories created during the operation
          of the PostgreSQL server, such as any file or directory beginning
-         with <filename>pgsql_tmp</filename>.
+         with <filename>pgsql_tmp</filename> and temporary relations.
         </para>
        </listitem>
        <listitem>
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<digits>_<digits>, or t<digits>_<digits>_<forkname> */
-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 t9999_999.1 t999_9999_vm t99999_99999_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");
+}
+
 # Make sure existing backup_label was ignored.
 isnt(slurp_file("$tempdir/backup/backup_label"),
 	'DONOTCOPY', 'existing backup_label not copied');
@@ -206,6 +226,19 @@ SKIP:
 	ok(-f "$pgdata/$tblspc1UnloggedPath",
 		'unlogged main fork in tablespace');
 
+	# Create files that look like temporary relations to ensure they are ignored
+	# in a tablespace.
+	my @tempRelationFiles = qw(t888_888 t888888_888888_vm.1);
+	my $tblSpc1Id = basename(dirname(dirname($node->safe_psql('postgres',
+		q{select pg_relation_filepath('test1')}))));
+
+	foreach my $filename (@tempRelationFiles)
+	{
+		append_to_file(
+			"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+			'TEMP_RELATION');
+	}
+
 	$node->command_fails(
 		[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
 		'plain format with tablespaces fails without tablespace mapping');
@@ -232,6 +265,20 @@ SKIP:
 	ok(!-f "$tempdir/tbackup/tblspc1/$tblspc1UnloggedBackupPath",
 		'unlogged main fork not in tablespace backup');
 
+	# Temp relations should not be copied.
+	foreach my $filename (@tempRelationFiles)
+	{
+		ok(!-f "$tempdir/tbackup/tblspc1/$tblSpc1Id/$postgresOid/$filename",
+		   "[tblspc1]/$postgresOid/$filename not copied");
+
+		# Also remove temp relation files or tablespace drop will fail.
+		my $filepath =
+			"$shorter_tempdir/tblspc1/$tblSpc1Id/$postgresOid/$filename";
+
+		unlink($filepath)
+			or BAIL_OUT("unable to unlink $filepath");
+	}
+
 	ok( -d "$tempdir/backup1/pg_replslot",
 		'pg_replslot symlink copied as directory');
 
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index 4244e7b1fd..e49b42ce86 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -124,6 +124,7 @@ extern void AtEOXact_Files(void);
 extern void AtEOSubXact_Files(bool isCommit, SubTransactionId mySubid,
 				  SubTransactionId parentSubid);
 extern void RemovePgTempFiles(void);
+extern bool looks_like_temp_rel_name(const char *name);
 
 extern int	pg_fsync(int fd);
 extern int	pg_fsync_no_writethrough(int fd);
-- 
2.14.3 (Apple Git-98)

#4Teodor Sigaev
teodor@sigaev.ru
In reply to: David Steele (#3)
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: teodor@sigaev.ru
WWW: http://www.sigaev.ru/

#5Stephen Frost
sfrost@snowman.net
In reply to: Teodor Sigaev (#4)
Re: PATCH: Exclude temp relations from base backup

Greetings,

* Teodor Sigaev (teodor@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

#6David Steele
david@pgmasters.net
In reply to: Stephen Frost (#5)
Re: PATCH: Exclude temp relations from base backup

On 3/26/18 1:06 PM, Stephen Frost wrote:

* Teodor Sigaev (teodor@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
david@pgmasters.net

#7Teodor Sigaev
teodor@sigaev.ru
In reply to: David Steele (#6)
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 (teodor@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: teodor@sigaev.ru
WWW: http://www.sigaev.ru/