Minor cleanup of pg_rewind.

hlinnaka · hlinnaka · commit 41457fcf970f · 2015-04-15T22:52:00.000+03:00
Update comments and function names to use the terms "source" and "target"
consistently. Some places were calling them remote and local instead, which
was confusing.

Fix incorrect comment in extractPageInfo on database creation record - it
was wrong on what happens for databases created in the target that don't
exist in source.
diff --git a/src/bin/pg_rewind/fetch.c b/src/bin/pg_rewind/fetch.c
@@ -26,10 +26,10 @@
 #include "filemap.h"
 
 void
-fetchRemoteFileList(void)
+fetchSourceFileList(void)
 {
 	if (datadir_source)
-		traverse_datadir(datadir_source, &process_remote_file);
+		traverse_datadir(datadir_source, &process_source_file);
 	else
 		libpqProcessFileList();
 }
diff --git a/src/bin/pg_rewind/fetch.h b/src/bin/pg_rewind/fetch.h
@@ -25,7 +25,7 @@
  * Common interface. Calls the copy or libpq method depending on global
  * config options.
  */
-extern void fetchRemoteFileList(void);
+extern void fetchSourceFileList(void);
 extern char *fetchFile(char *filename, size_t *filesize);
 extern void executeFileMap(void);
 
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
@@ -51,14 +51,14 @@ filemap_create(void)
 }
 
 /*
- * Callback for processing remote file list.
+ * Callback for processing source file list.
  *
  * This is called once for every file in the source server. We decide what
  * action needs to be taken for the file, depending on whether the file
  * exists in the target and whether the size matches.
  */
 void
-process_remote_file(const char *path, file_type_t type, size_t newsize,
+process_source_file(const char *path, file_type_t type, size_t newsize,
 					const char *link_target)
 {
 	bool		exists;
@@ -97,7 +97,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize,
 
 	snprintf(localpath, sizeof(localpath), "%s/%s", datadir_target, path);
 
-	/* Does the corresponding local file exist? */
+	/* Does the corresponding file exist in the target data dir? */
 	if (lstat(localpath, &statbuf) < 0)
 	{
 		if (errno != ENOENT)
@@ -185,18 +185,19 @@ process_remote_file(const char *path, file_type_t type, size_t newsize,
 				 *
 				 * If it's smaller in the target, it means that it has been
 				 * truncated in the target, or enlarged in the source, or
-				 * both. If it was truncated locally, we need to copy the
-				 * missing tail from the remote system. If it was enlarged in
-				 * the remote system, there will be WAL records in the remote
+				 * both. If it was truncated in the target, we need to copy the
+				 * missing tail from the source system. If it was enlarged in
+				 * the source system, there will be WAL records in the source
 				 * system for the new blocks, so we wouldn't need to copy them
 				 * here. But we don't know which scenario we're dealing with,
 				 * and there's no harm in copying the missing blocks now, so
 				 * do it now.
 				 *
-				 * If it's the same size, do nothing here. Any locally
-				 * modified blocks will be copied based on parsing the local
-				 * WAL, and any remotely modified blocks will be updated after
-				 * rewinding, when the remote WAL is replayed.
+				 * If it's the same size, do nothing here. Any blocks modified
+				 * in the target will be copied based on parsing the target
+				 * system's WAL, and any blocks modified in the source will be
+				 * updated after rewinding, when the source system's WAL is
+				 * replayed.
 				 */
 				oldsize = statbuf.st_size;
 				if (oldsize < newsize)
@@ -233,14 +234,15 @@ process_remote_file(const char *path, file_type_t type, size_t newsize,
 }
 
 /*
- * Callback for processing local file list.
+ * Callback for processing target file list.
  *
- * All remote files must be already processed before calling this. This only
- * marks local files that didn't exist in the remote system for deletion.
+ * All source files must be already processed before calling this. This only
+ * marks target data directory's files that didn't exist in the source for
+ * deletion.
  */
 void
-process_local_file(const char *path, file_type_t type, size_t oldsize,
-				   const char *link_target)
+process_target_file(const char *path, file_type_t type, size_t oldsize,
+					const char *link_target)
 {
 	bool		exists;
 	char		localpath[MAXPGPATH];
@@ -266,7 +268,7 @@ process_local_file(const char *path, file_type_t type, size_t oldsize,
 		if (map->nlist == 0)
 		{
 			/* should not happen */
-			pg_fatal("remote file list is empty\n");
+			pg_fatal("source file list is empty\n");
 		}
 
 		filemap_list_to_array(map);
@@ -288,7 +290,7 @@ process_local_file(const char *path, file_type_t type, size_t oldsize,
 	exists = (bsearch(&key_ptr, map->array, map->narray, sizeof(file_entry_t *),
 					  path_cmp) != NULL);
 
-	/* Remove any file or folder that doesn't exist in the remote system. */
+	/* Remove any file or folder that doesn't exist in the source system. */
 	if (!exists)
 	{
 		entry = pg_malloc(sizeof(file_entry_t));
@@ -313,16 +315,16 @@ process_local_file(const char *path, file_type_t type, size_t oldsize,
 	else
 	{
 		/*
-		 * We already handled all files that exist in the remote system in
-		 * process_remote_file().
+		 * We already handled all files that exist in the source system in
+		 * process_source_file().
 		 */
 	}
 }
 
 /*
- * This callback gets called while we read the old WAL, for every block that
- * have changed in the local system. It makes note of all the changed blocks
- * in the pagemap of the file.
+ * This callback gets called while we read the WAL in the target, for every
+ * block that have changed in the target system. It makes note of all the
+ * changed blocks in the pagemap of the file.
  */
 void
 process_block_change(ForkNumber forknum, RelFileNode rnode, BlockNumber blkno)
@@ -388,8 +390,8 @@ process_block_change(ForkNumber forknum, RelFileNode rnode, BlockNumber blkno)
 	{
 		/*
 		 * If we don't have any record of this file in the file map, it means
-		 * that it's a relation that doesn't exist in the remote system, and
-		 * it was subsequently removed in the local system, too. We can safely
+		 * that it's a relation that doesn't exist in the source system, and
+		 * it was subsequently removed in the target system, too. We can safely
 		 * ignore it.
 		 */
 	}
diff --git a/src/bin/pg_rewind/filemap.h b/src/bin/pg_rewind/filemap.h
@@ -62,8 +62,8 @@ typedef struct file_entry_t
 typedef struct filemap_t
 {
 	/*
-	 * New entries are accumulated to a linked list, in process_remote_file
-	 * and process_local_file.
+	 * New entries are accumulated to a linked list, in process_source_file
+	 * and process_target_file.
 	 */
 	file_entry_t *first;
 	file_entry_t *last;
@@ -94,9 +94,12 @@ extern void calculate_totals(void);
 extern void print_filemap(void);
 
 /* Functions for populating the filemap */
-extern void process_remote_file(const char *path, file_type_t type, size_t newsize, const char *link_target);
-extern void process_local_file(const char *path, file_type_t type, size_t newsize, const char *link_target);
-extern void process_block_change(ForkNumber forknum, RelFileNode rnode, BlockNumber blkno);
+extern void process_source_file(const char *path, file_type_t type,
+					size_t newsize, const char *link_target);
+extern void process_target_file(const char *path, file_type_t type,
+					size_t newsize, const char *link_target);
+extern void process_block_change(ForkNumber forknum, RelFileNode rnode,
+					 BlockNumber blkno);
 extern void filemap_finalize(void);
 
 #endif   /* FILEMAP_H */
diff --git a/src/bin/pg_rewind/libpq_fetch.c b/src/bin/pg_rewind/libpq_fetch.c
@@ -190,7 +190,7 @@ libpqProcessFileList(void)
 		else
 			type = FILE_TYPE_REGULAR;
 
-		process_remote_file(path, type, filesize, link_target);
+		process_source_file(path, type, filesize, link_target);
 	}
 }
 
diff --git a/src/bin/pg_rewind/logging.c b/src/bin/pg_rewind/logging.c
@@ -76,6 +76,9 @@ pg_log(eLogType type, const char *fmt,...)
 }
 
 
+/*
+ * Print an error message, and exit.
+ */
 void
 pg_fatal(const char *fmt,...)
 {
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
@@ -314,39 +314,37 @@ extractPageInfo(XLogReaderState *record)
 	{
 		/*
 		 * New databases can be safely ignored. It won't be present in the
-		 * remote system, so it will be copied in toto. There's one
-		 * corner-case, though: if a new, different, database is also created
-		 * in the remote system, we'll see that the files already exist and
-		 * not copy them. That's OK, though; WAL replay of creating the new
-		 * database, from the remote WAL, will re-copy the new database,
-		 * overwriting the database created in the local system.
+		 * source system, so it will be deleted. There's one corner-case,
+		 * though: if a new, different, database is also created in the
+		 * source system, we'll see that the files already exist and not copy
+		 * them. That's OK, though; WAL replay of creating the new database,
+		 * from the source systems's WAL, will re-copy the new database,
+		 * overwriting the database created in the target system.
 		 */
 	}
 	else if (rmid == RM_DBASE_ID && rminfo == XLOG_DBASE_DROP)
 	{
 		/*
 		 * An existing database was dropped. We'll see that the files don't
-		 * exist in local system, and copy them in toto from the remote
+		 * exist in the target data dir, and copy them in toto from the source
 		 * system. No need to do anything special here.
 		 */
 	}
 	else if (rmid == RM_SMGR_ID && rminfo == XLOG_SMGR_CREATE)
 	{
 		/*
-		 * We can safely ignore these. The local file will be removed, if it
-		 * doesn't exist in remote system. If a file with same name is created
-		 * in remote system, too, there will be WAL records for all the blocks
-		 * in it.
+		 * We can safely ignore these. The file will be removed from the
+		 * target, if it doesn't exist in source system. If a file with same
+		 * name is created in source system, too, there will be WAL records
+		 * for all the blocks in it.
 		 */
 	}
 	else if (rmid == RM_SMGR_ID && rminfo == XLOG_SMGR_TRUNCATE)
 	{
 		/*
-		 * We can safely ignore these. If a file is truncated locally, we'll
-		 * notice that when we compare the sizes, and will copy the missing
-		 * tail from remote system.
-		 *
-		 * TODO: But it would be nice to do some sanity cross-checking here..
+		 * We can safely ignore these. When we compare the sizes later on,
+		 * we'll notice that they differ, and copy the missing tail from
+		 * source system.
 		 */
 	}
 	else if (info & XLR_SPECIAL_REL_UPDATE)
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
@@ -263,13 +263,13 @@ main(int argc, char **argv)
 		   chkpttli);
 
 	/*
-	 * Build the filemap, by comparing the remote and local data directories.
+	 * Build the filemap, by comparing the source and target data directories.
 	 */
 	filemap_create();
 	pg_log(PG_PROGRESS, "reading source file list\n");
-	fetchRemoteFileList();
+	fetchSourceFileList();
 	pg_log(PG_PROGRESS, "reading target file list\n");
-	traverse_datadir(datadir_target, &process_local_file);
+	traverse_datadir(datadir_target, &process_target_file);
 
 	/*
 	 * Read the target WAL from last checkpoint before the point of fork, to

Original file line number	Diff line number	Diff line change
`@@ -26,10 +26,10 @@`
`26`	`26`	`#include "filemap.h"`
`27`	`27`
`28`	`28`	`void`
`29`		`-fetchRemoteFileList(void)`
	`29`	`+fetchSourceFileList(void)`
`30`	`30`	`{`
`31`	`31`	`if (datadir_source)`
`32`		`- traverse_datadir(datadir_source, &process_remote_file);`
	`32`	`+ traverse_datadir(datadir_source, &process_source_file);`
`33`	`33`	`else`
`34`	`34`	`libpqProcessFileList();`
`35`	`35`	`}`
Original file line number	Diff line number	Diff line change
`@@ -190,7 +190,7 @@ libpqProcessFileList(void)`
`190`	`190`	`else`
`191`	`191`	`type = FILE_TYPE_REGULAR;`
`192`	`192`
`193`		`- process_remote_file(path, type, filesize, link_target);`
	`193`	`+ process_source_file(path, type, filesize, link_target);`
`194`	`194`	`}`
`195`	`195`	`}`
`196`	`196`
Original file line number	Diff line number	Diff line change
`@@ -76,6 +76,9 @@ pg_log(eLogType type, const char *fmt,...)`
`76`	`76`	`}`
`77`	`77`
`78`	`78`
	`79`	`+/*`
	`80`	`+ * Print an error message, and exit.`
	`81`	`+ */`
`79`	`82`	`void`
`80`	`83`	`pg_fatal(const char *fmt,...)`
`81`	`84`	`{`