Hi,

I fixed some bugs.

On Thu, Dec 25, 2008 at 12:31 AM, Simon Riggs <si...@2ndquadrant.com> wrote:
>
> Can we change to IMMEDIATE when it we need the checkpoint?

Perhaps yes, though current patch doesn't care about it.
I'm not sure if we really need the feature. Yes, as you say,
I'd like to also listen to everybody else.

>
> What is bkpCount for?

So far, name of a backup history file consists of only
checkpoint redo location. But, in this patch, since some
backups use the same checkpoint, a backup history file
could be overwritten unfortunately. So, I introduced
bkpCount as ID of backups which use the same checkpoint.

> I think we should discuss whatever that is for
> separately. It isn't used in any if test, AFAICS.

Yes, this patch is testbed. We need to discuss more.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
? GNUmakefile
? config.log
? config.status
? contrib/make.log
? contrib/pgbench/pgbench
? src/Makefile.global
? src/backend/postgres
? src/backend/catalog/postgres.bki
? src/backend/catalog/postgres.description
? src/backend/catalog/postgres.shdescription
? src/backend/snowball/snowball_create.sql
? src/backend/utils/probes.h
? src/backend/utils/mb/conversion_procs/conversion_create.sql
? src/bin/initdb/initdb
? src/bin/pg_config/pg_config
? src/bin/pg_controldata/pg_controldata
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_dump/pg_restore
? src/bin/pg_resetxlog/pg_resetxlog
? src/bin/psql/psql
? src/bin/scripts/clusterdb
? src/bin/scripts/createdb
? src/bin/scripts/createlang
? src/bin/scripts/createuser
? src/bin/scripts/dropdb
? src/bin/scripts/droplang
? src/bin/scripts/dropuser
? src/bin/scripts/reindexdb
? src/bin/scripts/vacuumdb
? src/include/pg_config.h
? src/include/stamp-h
? src/interfaces/ecpg/compatlib/exports.list
? src/interfaces/ecpg/compatlib/libecpg_compat.so.3.1
? src/interfaces/ecpg/ecpglib/exports.list
? src/interfaces/ecpg/ecpglib/libecpg.so.6.1
? src/interfaces/ecpg/include/ecpg_config.h
? src/interfaces/ecpg/pgtypeslib/exports.list
? src/interfaces/ecpg/pgtypeslib/libpgtypes.so.3.1
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpq/exports.list
? src/interfaces/libpq/libpq.so.5.2
? src/port/pg_config_paths.h
? src/test/regress/log
? src/test/regress/pg_regress
? src/test/regress/results
? src/test/regress/testtablespace
? src/test/regress/tmp_check
? src/test/regress/expected/constraints.out
? src/test/regress/expected/copy.out
? src/test/regress/expected/create_function_1.out
? src/test/regress/expected/create_function_2.out
? src/test/regress/expected/largeobject.out
? src/test/regress/expected/largeobject_1.out
? src/test/regress/expected/misc.out
? src/test/regress/expected/tablespace.out
? src/test/regress/sql/constraints.sql
? src/test/regress/sql/copy.sql
? src/test/regress/sql/create_function_1.sql
? src/test/regress/sql/create_function_2.sql
? src/test/regress/sql/largeobject.sql
? src/test/regress/sql/misc.sql
? src/test/regress/sql/tablespace.sql
? src/timezone/zic
Index: src/backend/access/transam/xlog.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.324
diff -c -r1.324 xlog.c
*** src/backend/access/transam/xlog.c	17 Dec 2008 01:39:03 -0000	1.324
--- src/backend/access/transam/xlog.c	24 Dec 2008 18:13:45 -0000
***************
*** 295,300 ****
--- 295,302 ----
  	/* Protected by info_lck: */
  	XLogwrtRqst LogwrtRqst;
  	XLogwrtResult LogwrtResult;
+ 	uint32		bkpCount;		/* ID of bkp using the same ckpt */
+ 	bool		bkpForceCkpt;	/* reset full_page_writes since last ckpt? */
  	uint32		ckptXidEpoch;	/* nextXID & epoch of latest checkpoint */
  	TransactionId ckptXid;
  	XLogRecPtr	asyncCommitLSN; /* LSN of newest async commit */
***************
*** 318,323 ****
--- 320,332 ----
  static XLogCtlData *XLogCtl = NULL;
  
  /*
+  * We don't allow more than MAX_BKP_COUNT backups to use the same checkpoint.
+  * If XLogCtl->bkpCount > MAX_BKP_COUNT, we force new checkpoint at pg_standby
+  * even if there are all indispensable full pages since last checkpoint.
+  */
+ #define MAX_BKP_COUNT 256
+ 
+ /*
   * We maintain an image of pg_control in shared memory.
   */
  static ControlFileData *ControlFile = NULL;
***************
*** 6025,6036 ****
  	UpdateControlFile();
  	LWLockRelease(ControlFileLock);
  
! 	/* Update shared-memory copy of checkpoint XID/epoch */
  	{
  		/* use volatile pointer to prevent code rearrangement */
  		volatile XLogCtlData *xlogctl = XLogCtl;
  
  		SpinLockAcquire(&xlogctl->info_lck);
  		xlogctl->ckptXidEpoch = checkPoint.nextXidEpoch;
  		xlogctl->ckptXid = checkPoint.nextXid;
  		SpinLockRelease(&xlogctl->info_lck);
--- 6034,6050 ----
  	UpdateControlFile();
  	LWLockRelease(ControlFileLock);
  
! 	/* 
! 	 * Update shared-memory copy of checkpoint XID/epoch and reset the
! 	 * variables of backup ID/flag.
! 	 */
  	{
  		/* use volatile pointer to prevent code rearrangement */
  		volatile XLogCtlData *xlogctl = XLogCtl;
  
  		SpinLockAcquire(&xlogctl->info_lck);
+ 		xlogctl->bkpCount = 0;
+ 		xlogctl->bkpForceCkpt = true;
  		xlogctl->ckptXidEpoch = checkPoint.nextXidEpoch;
  		xlogctl->ckptXid = checkPoint.nextXid;
  		SpinLockRelease(&xlogctl->info_lck);
***************
*** 6502,6507 ****
--- 6516,6542 ----
  	}
  }
  
+ bool
+ assign_full_page_writes(bool newval, bool doit, GucSource source)
+ {
+ 	/*
+ 	 * If full_page_writes is reset, since all indispensable full pages
+ 	 * might not be written since last checkpoint, we force a checkpoint
+ 	 * at pg_start_backup. Only postmaster reset a shared-memory flag.
+ 	 */
+ 	if (doit && MyProcPid == PostmasterPid && fullPageWrites != newval)
+ 	{
+ 		/* use volatile pointer to prevent code rearrangement */
+ 		volatile XLogCtlData *xlogctl = XLogCtl;
+ 
+ 		SpinLockAcquire(&xlogctl->info_lck);
+ 		xlogctl->bkpForceCkpt = false;
+ 		SpinLockRelease(&xlogctl->info_lck);
+ 	}
+ 	
+ 	return true;
+ }
+ 
  
  /*
   * pg_start_backup: set up for taking an on-line backup dump
***************
*** 6519,6524 ****
--- 6554,6561 ----
  	char	   *backupidstr;
  	XLogRecPtr	checkpointloc;
  	XLogRecPtr	startpoint;
+ 	uint32		bkpCount;
+ 	bool		bkpForceCkpt;
  	pg_time_t	stamp_time;
  	char		strfbuf[128];
  	char		xlogfilename[MAXFNAMELEN];
***************
*** 6580,6593 ****
  	PG_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) 0);
  	{
  		/*
! 		 * Force a CHECKPOINT.	Aside from being necessary to prevent torn
! 		 * page problems, this guarantees that two successive backup runs will
! 		 * have different checkpoint positions and hence different history
! 		 * file names, even if nothing happened in between.
  		 *
  		 * We don't use CHECKPOINT_IMMEDIATE, hence this can take awhile.
  		 */
! 		RequestCheckpoint(CHECKPOINT_FORCE | CHECKPOINT_WAIT);
  
  		/*
  		 * Now we need to fetch the checkpoint record location, and also its
--- 6617,6657 ----
  	PG_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) 0);
  	{
  		/*
! 		 * Each backup might use the same checkpoint. Of course, since
! 		 * we cannot identify each only from a checkpoint positions,
! 		 * we use XLogCtl->bkpCount as ID. This identification prevents
! 		 * undesirable overwrite of a backup history file at least.
! 		 */
! 		{
! 			/* use volatile pointer to prevent code rearrangement */
! 			volatile XLogCtlData *xlogctl = XLogCtl;
! 			
! 			SpinLockAcquire(&xlogctl->info_lck);
! 			bkpCount		= ++xlogctl->bkpCount;
! 			bkpForceCkpt	= xlogctl->bkpForceCkpt;
! 			SpinLockRelease(&xlogctl->info_lck);
! 		}
! 
! 		/*
! 		 * If all indispensable full pages might not be written since
! 		 * last checkpoint, we force a CHECKPOINT now. This is necessary
! 		 * to prevent torn page problems.
  		 *
  		 * We don't use CHECKPOINT_IMMEDIATE, hence this can take awhile.
  		 */
! 		if (!fullPageWrites || !bkpForceCkpt || bkpCount > MAX_BKP_COUNT)
! 		{
! 			RequestCheckpoint(CHECKPOINT_FORCE | CHECKPOINT_WAIT);
! 
! 			{
! 				/* use volatile pointer to prevent code rearrangement */
! 				volatile XLogCtlData *xlogctl = XLogCtl;
! 				
! 				SpinLockAcquire(&xlogctl->info_lck);
! 				bkpCount		= ++xlogctl->bkpCount;
! 				SpinLockRelease(&xlogctl->info_lck);
! 			}
! 		}
  
  		/*
  		 * Now we need to fetch the checkpoint record location, and also its
***************
*** 6637,6644 ****
  					(errcode_for_file_access(),
  					 errmsg("could not create file \"%s\": %m",
  							BACKUP_LABEL_FILE)));
! 		fprintf(fp, "START WAL LOCATION: %X/%X (file %s)\n",
! 				startpoint.xlogid, startpoint.xrecoff, xlogfilename);
  		fprintf(fp, "CHECKPOINT LOCATION: %X/%X\n",
  				checkpointloc.xlogid, checkpointloc.xrecoff);
  		fprintf(fp, "START TIME: %s\n", strfbuf);
--- 6701,6708 ----
  					(errcode_for_file_access(),
  					 errmsg("could not create file \"%s\": %m",
  							BACKUP_LABEL_FILE)));
! 		fprintf(fp, "START WAL LOCATION: %X/%X (file %s id %X)\n",
! 				startpoint.xlogid, startpoint.xrecoff, xlogfilename, bkpCount);
  		fprintf(fp, "CHECKPOINT LOCATION: %X/%X\n",
  				checkpointloc.xlogid, checkpointloc.xrecoff);
  		fprintf(fp, "START TIME: %s\n", strfbuf);
***************
*** 6683,6688 ****
--- 6747,6753 ----
  {
  	XLogRecPtr	startpoint;
  	XLogRecPtr	stoppoint;
+ 	uint32		bkpCount;
  	pg_time_t	stamp_time;
  	char		strfbuf[128];
  	char		histfilepath[MAXPGPATH];
***************
*** 6753,6761 ****
  	 * Read and parse the START WAL LOCATION line (this code is pretty crude,
  	 * but we are not expecting any variability in the file format).
  	 */
! 	if (fscanf(lfp, "START WAL LOCATION: %X/%X (file %24s)%c",
  			   &startpoint.xlogid, &startpoint.xrecoff, startxlogfilename,
! 			   &ch) != 4 || ch != '\n')
  		ereport(ERROR,
  				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  				 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE)));
--- 6818,6826 ----
  	 * Read and parse the START WAL LOCATION line (this code is pretty crude,
  	 * but we are not expecting any variability in the file format).
  	 */
! 	if (fscanf(lfp, "START WAL LOCATION: %X/%X (file %24s id %X)%c",
  			   &startpoint.xlogid, &startpoint.xrecoff, startxlogfilename,
! 			   &bkpCount, &ch) != 5 || ch != '\n')
  		ereport(ERROR,
  				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  				 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE)));
***************
*** 6765,6779 ****
  	 */
  	XLByteToSeg(startpoint, _logId, _logSeg);
  	BackupHistoryFilePath(histfilepath, ThisTimeLineID, _logId, _logSeg,
! 						  startpoint.xrecoff % XLogSegSize);
  	fp = AllocateFile(histfilepath, "w");
  	if (!fp)
  		ereport(ERROR,
  				(errcode_for_file_access(),
  				 errmsg("could not create file \"%s\": %m",
  						histfilepath)));
! 	fprintf(fp, "START WAL LOCATION: %X/%X (file %s)\n",
! 			startpoint.xlogid, startpoint.xrecoff, startxlogfilename);
  	fprintf(fp, "STOP WAL LOCATION: %X/%X (file %s)\n",
  			stoppoint.xlogid, stoppoint.xrecoff, stopxlogfilename);
  	/* transfer remaining lines from label to history file */
--- 6830,6844 ----
  	 */
  	XLByteToSeg(startpoint, _logId, _logSeg);
  	BackupHistoryFilePath(histfilepath, ThisTimeLineID, _logId, _logSeg,
! 						  startpoint.xrecoff % XLogSegSize, bkpCount);
  	fp = AllocateFile(histfilepath, "w");
  	if (!fp)
  		ereport(ERROR,
  				(errcode_for_file_access(),
  				 errmsg("could not create file \"%s\": %m",
  						histfilepath)));
! 	fprintf(fp, "START WAL LOCATION: %X/%X (file %s id %X)\n",
! 			startpoint.xlogid, startpoint.xrecoff, startxlogfilename, bkpCount);
  	fprintf(fp, "STOP WAL LOCATION: %X/%X (file %s)\n",
  			stoppoint.xlogid, stoppoint.xrecoff, stopxlogfilename);
  	/* transfer remaining lines from label to history file */
***************
*** 6822,6828 ****
  
  	XLByteToSeg(startpoint, _logId, _logSeg);
  	BackupHistoryFileName(histfilename, ThisTimeLineID, _logId, _logSeg,
! 						  startpoint.xrecoff % XLogSegSize);
  
  	seconds_before_warning = 60;
  	waits = 0;
--- 6887,6893 ----
  
  	XLByteToSeg(startpoint, _logId, _logSeg);
  	BackupHistoryFileName(histfilename, ThisTimeLineID, _logId, _logSeg,
! 						  startpoint.xrecoff % XLogSegSize, bkpCount);
  
  	seconds_before_warning = 60;
  	waits = 0;
***************
*** 7060,7065 ****
--- 7125,7131 ----
  {
  	XLogRecPtr	startpoint;
  	XLogRecPtr	stoppoint;
+ 	uint32		bkpCount;
  	char		histfilename[MAXFNAMELEN];
  	char		histfilepath[MAXPGPATH];
  	char		startxlogfilename[MAXFNAMELEN];
***************
*** 7094,7102 ****
  	 * is pretty crude, but we are not expecting any variability in the file
  	 * format).
  	 */
! 	if (fscanf(lfp, "START WAL LOCATION: %X/%X (file %08X%16s)%c",
  			   &startpoint.xlogid, &startpoint.xrecoff, &tli,
! 			   startxlogfilename, &ch) != 5 || ch != '\n')
  		ereport(FATAL,
  				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  				 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE)));
--- 7160,7168 ----
  	 * is pretty crude, but we are not expecting any variability in the file
  	 * format).
  	 */
! 	if (fscanf(lfp, "START WAL LOCATION: %X/%X (file %08X%16s id %X)%c",
  			   &startpoint.xlogid, &startpoint.xrecoff, &tli,
! 			   startxlogfilename, &bkpCount, &ch) != 6 || ch != '\n')
  		ereport(FATAL,
  				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  				 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE)));
***************
*** 7117,7129 ****
  	 */
  	XLByteToSeg(startpoint, _logId, _logSeg);
  	BackupHistoryFileName(histfilename, tli, _logId, _logSeg,
! 						  startpoint.xrecoff % XLogSegSize);
  
  	if (InArchiveRecovery)
  		RestoreArchivedFile(histfilepath, histfilename, "RECOVERYHISTORY", 0);
  	else
  		BackupHistoryFilePath(histfilepath, tli, _logId, _logSeg,
! 							  startpoint.xrecoff % XLogSegSize);
  
  	fp = AllocateFile(histfilepath, "r");
  	if (fp)
--- 7183,7195 ----
  	 */
  	XLByteToSeg(startpoint, _logId, _logSeg);
  	BackupHistoryFileName(histfilename, tli, _logId, _logSeg,
! 						  startpoint.xrecoff % XLogSegSize, bkpCount);
  
  	if (InArchiveRecovery)
  		RestoreArchivedFile(histfilepath, histfilename, "RECOVERYHISTORY", 0);
  	else
  		BackupHistoryFilePath(histfilepath, tli, _logId, _logSeg,
! 							  startpoint.xrecoff % XLogSegSize, bkpCount);
  
  	fp = AllocateFile(histfilepath, "r");
  	if (fp)
***************
*** 7131,7139 ****
  		/*
  		 * Parse history file to identify stop point.
  		 */
! 		if (fscanf(fp, "START WAL LOCATION: %X/%X (file %24s)%c",
  				   &startpoint.xlogid, &startpoint.xrecoff, startxlogfilename,
! 				   &ch) != 4 || ch != '\n')
  			ereport(FATAL,
  					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  					 errmsg("invalid data in file \"%s\"", histfilename)));
--- 7197,7205 ----
  		/*
  		 * Parse history file to identify stop point.
  		 */
! 		if (fscanf(fp, "START WAL LOCATION: %X/%X (file %24s id %X)%c",
  				   &startpoint.xlogid, &startpoint.xrecoff, startxlogfilename,
! 				   &bkpCount, &ch) != 5 || ch != '\n')
  			ereport(FATAL,
  					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
  					 errmsg("invalid data in file \"%s\"", histfilename)));
Index: src/backend/postmaster/pgarch.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/postmaster/pgarch.c,v
retrieving revision 1.38
diff -c -r1.38 pgarch.c
*** src/backend/postmaster/pgarch.c	11 Jan 2008 00:54:08 -0000	1.38
--- src/backend/postmaster/pgarch.c	24 Dec 2008 18:14:22 -0000
***************
*** 66,72 ****
   * ----------
   */
  #define MIN_XFN_CHARS	16
! #define MAX_XFN_CHARS	40
  #define VALID_XFN_CHARS "0123456789ABCDEF.history.backup"
  
  #define NUM_ARCHIVE_RETRIES 3
--- 66,72 ----
   * ----------
   */
  #define MIN_XFN_CHARS	16
! #define MAX_XFN_CHARS	49
  #define VALID_XFN_CHARS "0123456789ABCDEF.history.backup"
  
  #define NUM_ARCHIVE_RETRIES 3
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.483
diff -c -r1.483 guc.c
*** src/backend/utils/misc/guc.c	13 Dec 2008 19:13:44 -0000	1.483
--- src/backend/utils/misc/guc.c	24 Dec 2008 18:15:12 -0000
***************
*** 57,62 ****
--- 57,63 ----
  #include "regex/regex.h"
  #include "storage/bufmgr.h"
  #include "storage/fd.h"
+ #include "storage/spin.h"
  #include "tcop/tcopprot.h"
  #include "tsearch/ts_cache.h"
  #include "utils/builtins.h"
***************
*** 713,719 ****
  						 "is possible.")
  		},
  		&fullPageWrites,
! 		true, NULL, NULL
  	},
  	{
  		{"silent_mode", PGC_POSTMASTER, LOGGING_WHEN,
--- 714,720 ----
  						 "is possible.")
  		},
  		&fullPageWrites,
! 		true, assign_full_page_writes, NULL
  	},
  	{
  		{"silent_mode", PGC_POSTMASTER, LOGGING_WHEN,
Index: src/include/access/xlog_internal.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/access/xlog_internal.h,v
retrieving revision 1.24
diff -c -r1.24 xlog_internal.h
*** src/include/access/xlog_internal.h	11 Aug 2008 11:05:11 -0000	1.24
--- src/include/access/xlog_internal.h	24 Dec 2008 18:15:29 -0000
***************
*** 215,225 ****
  #define StatusFilePath(path, xlog, suffix)	\
  	snprintf(path, MAXPGPATH, XLOGDIR "/archive_status/%s%s", xlog, suffix)
  
! #define BackupHistoryFileName(fname, tli, log, seg, offset) \
! 	snprintf(fname, MAXFNAMELEN, "%08X%08X%08X.%08X.backup", tli, log, seg, offset)
  
! #define BackupHistoryFilePath(path, tli, log, seg, offset)	\
! 	snprintf(path, MAXPGPATH, XLOGDIR "/%08X%08X%08X.%08X.backup", tli, log, seg, offset)
  
  
  /*
--- 215,225 ----
  #define StatusFilePath(path, xlog, suffix)	\
  	snprintf(path, MAXPGPATH, XLOGDIR "/archive_status/%s%s", xlog, suffix)
  
! #define BackupHistoryFileName(fname, tli, log, seg, offset, id)			\
! 	snprintf(fname, MAXFNAMELEN, "%08X%08X%08X.%08X.%08X.backup", tli, log, seg, offset, id)
  
! #define BackupHistoryFilePath(path, tli, log, seg, offset, id)				\
! 	snprintf(path, MAXPGPATH, XLOGDIR "/%08X%08X%08X.%08X.%08X.backup", tli, log, seg, offset, id)
  
  
  /*
Index: src/include/utils/guc.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc.h,v
retrieving revision 1.99
diff -c -r1.99 guc.h
*** src/include/utils/guc.h	19 Nov 2008 01:10:23 -0000	1.99
--- src/include/utils/guc.h	24 Dec 2008 18:15:33 -0000
***************
*** 308,312 ****
--- 308,314 ----
  /* in access/transam/xlog.c */
  extern bool assign_xlog_sync_method(int newval,
  				bool doit, GucSource source);
+ extern bool assign_full_page_writes(bool newval,
+ 				bool doit, GucSource source);
  
  #endif   /* GUC_H */
-- 
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