Push details of greenplum options into options_gp.c

contrib/pg_upgrade/controldata.c

@@ -726,20 +726,20 @@ check_control_data(ControlData *oldctrl,
 	 */
 	if (oldctrl->data_checksum_version == 0 &&
 		newctrl->data_checksum_version != 0 &&
-		greenplum_user_opts.checksum_mode != CHECKSUM_ADD)
+		!is_checksum_mode(CHECKSUM_ADD))
 		pg_fatal("old cluster does not use data checksums but the new one does\n");
 	else if (oldctrl->data_checksum_version != 0 &&
 			 newctrl->data_checksum_version == 0 &&
-			 greenplum_user_opts.checksum_mode != CHECKSUM_REMOVE)
+			 !is_checksum_mode(CHECKSUM_REMOVE))
 		pg_fatal("old cluster uses data checksums but the new one does not\n");
 	else if (oldctrl->data_checksum_version == newctrl->data_checksum_version &&
-			 greenplum_user_opts.checksum_mode != CHECKSUM_NONE)
+			 !is_checksum_mode(CHECKSUM_NONE))
 		pg_fatal("old and new cluster data checksum configuration match, cannot %s data checksums\n",
-				 (greenplum_user_opts.checksum_mode == CHECKSUM_ADD ? "add" : "remove"));
-	else if (oldctrl->data_checksum_version != 0 && greenplum_user_opts.checksum_mode == CHECKSUM_ADD)
+				 (is_checksum_mode(CHECKSUM_ADD) ? "add" : "remove"));
+	else if (oldctrl->data_checksum_version != 0 && is_checksum_mode(CHECKSUM_ADD))
 		pg_fatal("--add-checksum option not supported for old cluster which uses data checksums\n");
 	else if (oldctrl->data_checksum_version != newctrl->data_checksum_version
-			 && greenplum_user_opts.checksum_mode == CHECKSUM_NONE)
+			 && is_checksum_mode(CHECKSUM_NONE))
 		pg_fatal("old and new cluster pg_controldata checksum versions do not match\n");
 }
 

contrib/pg_upgrade/greenplum/file_gp.c

@@ -42,7 +42,7 @@ rewriteHeapPageChecksum(const char *fromfile, const char *tofile,
 	 * transfer_relfile() should never call us unless requested by the data
 	 * checksum option but better doublecheck before we start rewriting data.
 	 */
-	if (greenplum_user_opts.checksum_mode == CHECKSUM_NONE)
+	if (is_checksum_mode(CHECKSUM_NONE))
 		return "incorrect checksum configuration detected.\n";
 
 	if ((src_fd = open(fromfile, O_RDONLY | PG_BINARY, 0)) < 0)
@@ -78,7 +78,7 @@ rewriteHeapPageChecksum(const char *fromfile, const char *tofile,
 
 		if (!PageIsNew(buf))
 		{
-			if (greenplum_user_opts.checksum_mode == CHECKSUM_ADD)
+			if (is_checksum_mode(CHECKSUM_ADD))
 				((PageHeader) buf)->pd_checksum = pg_checksum_page(buf, blkno);
 			else
 				memset(&(((PageHeader) buf)->pd_checksum), 0, sizeof(uint16));

contrib/pg_upgrade/greenplum/gpdb4_heap_convert.c

@@ -278,7 +278,7 @@ flush_overflow_page(void)
 {
 	if (!PageIsNew(overflow_buf))
 	{
-		if (greenplum_user_opts.checksum_mode == CHECKSUM_ADD)
+		if (is_checksum_mode(CHECKSUM_ADD))
 			((PageHeader) overflow_buf)->pd_checksum =
 				pg_checksum_page(overflow_buf, overflow_blkno);
 
@@ -481,7 +481,7 @@ convert_gpdb4_heap_file(const char *src, const char *dst,
 		 * retaining an existing checksum like for upgrades from 5.x. If we're
 		 * not adding them we want a zeroed out portion in the header
 		 */
-		if (greenplum_user_opts.checksum_mode == CHECKSUM_ADD)
+		if (is_checksum_mode(CHECKSUM_ADD))
 			((PageHeader) buf)->pd_checksum = pg_checksum_page(buf, blkno);
 		else
 			memset(&(((PageHeader) buf)->pd_checksum), 0, sizeof(uint16));

contrib/pg_upgrade/greenplum/option_gp.c

 #include "pg_upgrade_greenplum.h"
 
-GreenplumUserOpts greenplum_user_opts;
+typedef enum
+{
+	DISPATCHER = 0,
+	SEGMENT
+} segmentMode;
+
+typedef struct {
+	bool progress;
+	segmentMode segment_mode;
+	checksumMode checksum_mode;
+	char *old_tablespace_file_path;
+} GreenplumUserOpts;
+
+static GreenplumUserOpts greenplum_user_opts;
 
 void
 initialize_greenplum_user_options(void)
 {
 	greenplum_user_opts.segment_mode = SEGMENT;
+	greenplum_user_opts.old_tablespace_file_path = NULL;
 }
 
 bool
-process_greenplum_option(int option, char *option_value)
+process_greenplum_option(greenplumOption option, char *option_value)
 {
 	switch (option)
 	{
@@ -56,8 +70,37 @@ process_greenplum_option(int option, char *option_value)
 	return true;
 }
 
+void
+validate_greenplum_options(void)
+{
+	if (old_cluster.gp_dbid == GP_DBID_NOT_SET)
+		pg_fatal("--old-gp-dbid must be set\n");
+
+	if (new_cluster.gp_dbid == GP_DBID_NOT_SET)
+		pg_fatal("--new-gp-dbid must be set\n");
+
+	if (greenplum_user_opts.old_tablespace_file_path) {
+		populate_old_cluster_with_old_tablespaces(
+			&old_cluster,
+			greenplum_user_opts.old_tablespace_file_path);
+	}
+}
+
 bool
 is_greenplum_dispatcher_mode()
 {
 	return greenplum_user_opts.segment_mode == DISPATCHER;
-}
\ No newline at end of file
+}
+
+bool
+is_checksum_mode(checksumMode mode)
+{
+	return mode == greenplum_user_opts.checksum_mode;
+}
+
+bool
+is_show_progress_mode(void)
+{
+	return greenplum_user_opts.progress;
+}
+

contrib/pg_upgrade/greenplum/pg_upgrade_greenplum.h

@@ -34,36 +34,25 @@ typedef enum
 	CHECKSUM_REMOVE
 } checksumMode;
 
-typedef enum
-{
-	DISPATCHER = 0,
-	SEGMENT
-} segmentMode;
-
-typedef struct {
-	bool progress;
-	segmentMode segment_mode;
-	checksumMode checksum_mode;
-	char *old_tablespace_file_path;
-} GreenplumUserOpts;
-
-#define GREENPLUM_MODE_OPTION 1
-#define GREENPLUM_PROGRESS_OPTION 2
-#define GREENPLUM_ADD_CHECKSUM_OPTION 3
-#define GREENPLUM_REMOVE_CHECKSUM_OPTION 4
-#define GREENPLUM_OLD_GP_DBID 5
-#define GREENPLUM_NEW_GP_DBID 6
-#define GREENPLUM_OLD_TABLESPACES_FILE 7
+typedef enum {
+	GREENPLUM_MODE_OPTION = 1,
+	GREENPLUM_PROGRESS_OPTION = 2,
+	GREENPLUM_ADD_CHECKSUM_OPTION = 3,
+	GREENPLUM_REMOVE_CHECKSUM_OPTION = 4,
+	GREENPLUM_OLD_GP_DBID = 5,
+	GREENPLUM_NEW_GP_DBID = 6,
+	GREENPLUM_OLD_TABLESPACES_FILE = 7
+} greenplumOption;
 
 
 #define GREENPLUM_OPTIONS \
 	{"mode", required_argument, NULL, GREENPLUM_MODE_OPTION}, \
 	{"progress", no_argument, NULL, GREENPLUM_PROGRESS_OPTION}, \
 	{"add-checksum", no_argument, NULL, GREENPLUM_ADD_CHECKSUM_OPTION}, \
-	{"remove-checksum", no_argument, NULL, GREENPLUM_REMOVE_CHECKSUM_OPTION},
-	{"old-gp-dbid", required_argument, NULL, 5},
-	{"new-gp-dbid", required_argument, NULL, 6},
-	{"old-tablespaces-file", required_argument, NULL, 7},
+	{"remove-checksum", no_argument, NULL, GREENPLUM_REMOVE_CHECKSUM_OPTION}, \
+	{"old-gp-dbid", required_argument, NULL, GREENPLUM_OLD_GP_DBID}, \
+	{"new-gp-dbid", required_argument, NULL, GREENPLUM_NEW_GP_DBID}, \
+	{"old-tablespaces-file", required_argument, NULL, GREENPLUM_OLD_TABLESPACES_FILE},
 
 #define GREENPLUM_USAGE "\
       --mode=TYPE               designate node type to upgrade, \"segment\" or \"dispatcher\" (default \"segment\")\n\
@@ -75,11 +64,16 @@ typedef struct {
       --old-tablespaces-file    file containing the tablespaces from an old gpdb five cluster\n\
 "
 
+#define GP_DBID_NOT_SET -1
+
+
 /* option_gp.c */
-extern GreenplumUserOpts greenplum_user_opts;
 void initialize_greenplum_user_options(void);
-bool process_greenplum_option(int option, char *option_value);
+bool process_greenplum_option(greenplumOption option, char *option_value);
 bool is_greenplum_dispatcher_mode(void);
+bool is_checksum_mode(checksumMode mode);
+bool is_show_progress_mode(void);
+void validate_greenplum_options(void);
 
 /* aotable.c */
 

contrib/pg_upgrade/greenplum/reporting.c

@@ -76,7 +76,7 @@ report_progress(ClusterInfo *cluster, progress_type op, char *fmt,...)
 	char			filename[MAXPGPATH];
 	unsigned long	ts;
 
-	if (!greenplum_user_opts.progress)
+	if (!is_show_progress_mode())
 		return;
 
 	ts = epoch_us();
@@ -113,7 +113,7 @@ close_progress(void)
 	char	old[MAXPGPATH];
 	char	new[MAXPGPATH];
 
-	if (!greenplum_user_opts.progress || !progress_file)
+	if (!is_show_progress_mode() || !progress_file)
 		return;
 
 	snprintf(old, sizeof(old), "%d.inprogress", progress_id);

contrib/pg_upgrade/option.c

@@ -27,7 +27,6 @@ static void check_required_directory(char **dirpath,
 						 const char *envVarName, bool useCwd,
 						 const char *cmdLineOption, const char *description);
 #define FIX_DEFAULT_READ_ONLY "-c default_transaction_read_only=false"
-#define GP_DBID_NOT_SET -1
 
 UserOpts	user_opts;
 
@@ -70,8 +69,6 @@ parseCommandLine(int argc, char *argv[])
 	time_t		run_time = time(NULL);
 
 	user_opts.transfer_mode = TRANSFER_MODE_COPY;
-	user_opts.old_tablespace_file_path = NULL;
-
 	old_cluster.gp_dbid = GP_DBID_NOT_SET;
 	new_cluster.gp_dbid = GP_DBID_NOT_SET;
 	old_cluster.old_tablespace_file_contents = NULL;
@@ -251,20 +248,10 @@ parseCommandLine(int argc, char *argv[])
 
 	/* Ensure we are only adding checksums in copy mode */
 	if (user_opts.transfer_mode != TRANSFER_MODE_COPY &&
-		greenplum_user_opts.checksum_mode != CHECKSUM_NONE)
+		!is_checksum_mode(CHECKSUM_NONE))
 		pg_log(PG_FATAL, "Adding and removing checksums only supported in copy mode.\n");
 
-	if (old_cluster.gp_dbid == GP_DBID_NOT_SET)
-		pg_fatal("--old-gp-dbid must be set\n");
-
-	if (new_cluster.gp_dbid == GP_DBID_NOT_SET)
-		pg_fatal("--new-gp-dbid must be set\n");
-
-	if (user_opts.old_tablespace_file_path) {
-		populate_old_cluster_with_old_tablespaces(
-			&old_cluster,
-			user_opts.old_tablespace_file_path);
-	}
+	validate_greenplum_options();
 }
 
 

contrib/pg_upgrade/relfilenode.c

@@ -359,7 +359,7 @@ transfer_relfile_segment(int segno, pageCnvCtx *pageConverter, FileNameMap *map,
 
 		if (user_opts.transfer_mode == TRANSFER_MODE_COPY)
 		{
-			if (greenplum_user_opts.checksum_mode != CHECKSUM_NONE && map->type == HEAP)
+			if (!is_checksum_mode(CHECKSUM_NONE) && map->type == HEAP)
 			{
 				pg_log(PG_VERBOSE, "copying and checksumming \"%s\" to \"%s\"\n", old_file, new_file);
 				if ((msg = rewriteHeapPageChecksum(old_file, new_file, map->nspname, map->relname)))