Fix WAL segment file generation

Michael Paquier · Michael Paquier · commit ced83f77035f · 2013-12-25T08:17:01.000+09:00
Name file of WAL segment was generated using the API of xlog_internal.h
called XlogFileName, based on XLogSegNo and not XLogRecPtr as the
previous code assumed. This leaded to backup incorrect, actually too
many WAL files in the archive code path because the analysis was based
on a name completely fucked up. This commit fixes at the same time an
issue in search_next_wal where the function could loop for a too long
amount of time, eating much CPU when looking for the next WAL file.

Regression tests are passing cleanly with this patch.
diff --git a/backup.c b/backup.c
@@ -433,7 +433,7 @@ do_backup_arclog(parray *backup_list)
 	char		prev_file_txt[MAXPGPATH];
 	pgBackup   *prev_backup;
 	int64		arclog_write_bytes = 0;
-	char		last_wal[MAXPGPATH];
+	char		last_wal[MAXFNAMELEN];
 
 	Assert(current.backup_mode == BACKUP_MODE_ARCHIVE ||
 		   current.backup_mode == BACKUP_MODE_INCREMENTAL ||
@@ -485,7 +485,7 @@ do_backup_arclog(parray *backup_list)
 	dir_list_file(files, arclog_path, NULL, true, false);
 
 	/* remove WALs archived after pg_stop_backup()/pg_switch_xlog() */
-	xlog_fname(last_wal, lengthof(last_wal), current.tli, &current.stop_lsn);
+	xlog_fname(last_wal, current.tli, current.stop_lsn);
 	for (i = 0; i < parray_num(files); i++)
 	{
 		pgFile *file = (pgFile *) parray_get(files, i);
@@ -1023,7 +1023,7 @@ wait_for_archive(pgBackup *backup, const char *sql)
 	}
 
 	/* As well as WAL file name */
-	XLogFileName(file_name, tli, lsn);
+	xlog_fname(file_name, tli, lsn);
 
 	snprintf(ready_path, lengthof(ready_path),
 		"%s/pg_xlog/archive_status/%s.ready", pgdata,
diff --git a/pg_rman.h b/pg_rman.h
@@ -307,7 +307,7 @@ extern int pgFileCompareMtimeDesc(const void *f1, const void *f2);
 
 /* in xlog.c */
 extern bool xlog_is_complete_wal(const pgFile *file);
-extern void xlog_fname(char *fname, size_t len, TimeLineID tli, XLogRecPtr *lsn);
+extern void xlog_fname(char *fname, TimeLineID tli, XLogRecPtr lsn);
 
 /* in data.c */
 extern bool backup_data_file(const char *from_root, const char *to_root,
diff --git a/restore.c b/restore.c
@@ -992,7 +992,7 @@ search_next_wal(const char *path, XLogRecPtr *need_lsn, parray *timelines)
 		{
 			pgTimeLine *timeline = (pgTimeLine *) parray_get(timelines, i);
 
-			XLogFileName(xlogfname, timeline->tli, *need_lsn);
+			xlog_fname(xlogfname, timeline->tli, *need_lsn);
 			join_path_components(xlogpath, path, xlogfname);
 
 			if (stat(xlogpath, &st) == 0)
@@ -1021,8 +1021,8 @@ search_next_wal(const char *path, XLogRecPtr *need_lsn, parray *timelines)
 			parray_remove(timelines, i + 1);
 		/* XXX: should we add a linebreak when we find a timeline? */
 
-		/* Move to next xlog record */
-		(*need_lsn)++;
+		/* Move to next xlog segment */
+		*need_lsn += XLogSegSize;
 	}
 }
 
diff --git a/xlog.c b/xlog.c
@@ -73,9 +73,9 @@ xlog_is_complete_wal(const pgFile *file)
  * based on XLogFileName() in xlog_internal.h
  */
 void
-xlog_fname(char *fname, size_t len, TimeLineID tli, XLogRecPtr *lsn)
+xlog_fname(char *fname, TimeLineID tli, XLogRecPtr lsn)
 {
-	snprintf(fname, len, "%08X%08X%08X", tli,
-			 (uint32) (*lsn / XLogSegSize),
-			 (uint32) (*lsn % XLogSegSize));
+	snprintf(fname, MAXFNAMELEN, "%08X%08X%08X", tli,
+			 (uint32) (lsn >> 32),
+			 (uint32) (lsn / XLogSegSize));
 }

Original file line number	Diff line number	Diff line change
`@@ -992,7 +992,7 @@ search_next_wal(const char path, XLogRecPtr need_lsn, parray *timelines)`
`992`	`992`	`{`
`993`	`993`	`pgTimeLine timeline = (pgTimeLine ) parray_get(timelines, i);`
`994`	`994`
`995`		`- XLogFileName(xlogfname, timeline->tli, *need_lsn);`
	`995`	`+ xlog_fname(xlogfname, timeline->tli, *need_lsn);`
`996`	`996`	`join_path_components(xlogpath, path, xlogfname);`
`997`	`997`
`998`	`998`	`if (stat(xlogpath, &st) == 0)`
`@@ -1021,8 +1021,8 @@ search_next_wal(const char path, XLogRecPtr need_lsn, parray *timelines)`
`1021`	`1021`	`parray_remove(timelines, i + 1);`
`1022`	`1022`	`/* XXX: should we add a linebreak when we find a timeline? */`
`1023`	`1023`
`1024`		`- /* Move to next xlog record */`
`1025`		`- (*need_lsn)++;`
	`1024`	`+ /* Move to next xlog segment */`
	`1025`	`+ *need_lsn += XLogSegSize;`
`1026`	`1026`	`}`
`1027`	`1027`	`}`
`1028`	`1028`
Original file line number	Diff line number	Diff line change
`@@ -73,9 +73,9 @@ xlog_is_complete_wal(const pgFile *file)`
`73`	`73`	`* based on XLogFileName() in xlog_internal.h`
`74`	`74`	`*/`
`75`	`75`	`void`
`76`		`-xlog_fname(char fname, size_t len, TimeLineID tli, XLogRecPtr lsn)`
	`76`	`+xlog_fname(char *fname, TimeLineID tli, XLogRecPtr lsn)`
`77`	`77`	`{`
`78`		`- snprintf(fname, len, "%08X%08X%08X", tli,`
`79`		`- (uint32) (*lsn / XLogSegSize),`
`80`		`- (uint32) (*lsn % XLogSegSize));`
	`78`	`+ snprintf(fname, MAXFNAMELEN, "%08X%08X%08X", tli,`
	`79`	`+ (uint32) (lsn >> 32),`
	`80`	`+ (uint32) (lsn / XLogSegSize));`
`81`	`81`	`}`