On Tue, Jan 28, 2014 at 5:51 PM, Magnus Hagander <mag...@hagander.net> wrote:
> On Tue, Jan 28, 2014 at 6:11 AM, Amit Kapila <amit.kapil...@gmail.com>
> wrote:
>>
>> On Tue, Jan 28, 2014 at 9:26 AM, Fujii Masao <masao.fu...@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.

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
*** 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
  {
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to