Thanks, You have right. It is problem with silent skip some option in set_config_option function when PGC_SIGHUP is running context.

I fix it and I attach third version of patch.

      Zdenek



Joachim Wieland wrote:
Zdenek,

On Wed, May 31, 2006 at 06:13:04PM +0200, Zdenek Kotala wrote:
Joachim, could you explain me second point? I cannot determine described problem. By my opinion my patch does not change this behavior.

I guess what I saw was another phenomenon:

I do the following:

- vi postgresql.conf => "allow_system_table_mods = true"
- start postmaster
- vi postgresql.conf => "# allow_system_table_mods = true" (commented)
- killall -HUP postmaster

Then I get _no_ message. After another killall -HUP I do indeed get a
message. So I don't get it just for the first time which is strange, do you
see that as well?

However the message I get is that it got reset to its default value which is
wrong because its a PGC_POSTMASTER variable that can only be set at server
start (set_config_option() returns true in this case as well).

Consequently I expect to get it for every other signal I send (because the
old value is still active and differs from what is in the configuration
file).


Joachim


Index: src/backend/utils/misc/guc-file.l
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.37
diff -c -r1.37 guc-file.l
*** src/backend/utils/misc/guc-file.l	7 Mar 2006 01:03:12 -0000	1.37
--- src/backend/utils/misc/guc-file.l	1 Jun 2006 14:46:56 -0000
***************
*** 112,119 ****
  void
  ProcessConfigFile(GucContext context)
  {
! 	int			elevel;
  	struct name_value_pair *item, *head, *tail;
  
  	Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
  
--- 112,120 ----
  void
  ProcessConfigFile(GucContext context)
  {
! 	int			elevel, i;
  	struct name_value_pair *item, *head, *tail;
+ 	char	   *env;
  
  	Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
  
***************
*** 150,155 ****
--- 151,207 ----
  						  PGC_S_FILE, false, true);
  	}
  
+ 	if( context == PGC_SIGHUP)
+ 	{
+ 	/* Revert all "untouched" options with reset source PGC_S_FILE to 
+      * default/boot value. 
+      */
+ 	for (i = 0; i < num_guc_variables; i++)
+ 	{
+ 		struct config_generic *gconf = guc_variables[i];
+ 		if ( gconf->context != PGC_INTERNAL && gconf->context != PGC_POSTMASTER && 
+ 			!( gconf->context == PGC_BACKEND && IsUnderPostmaster) &&
+ 			 (gconf->reset_source == PGC_S_FILE) && ( (gconf->status & GUC_JUST_RELOAD) == 0) )
+ 		{
+ 			if( set_config_option(gconf->name, NULL, context,
+ 				  PGC_S_FILE, false, true) )
+ 			{
+ 				GucStack   *stack;
+ 				/* set correctly source */
+ 				gconf->reset_source = PGC_S_DEFAULT;
+ 				for (stack = gconf->stack; stack; stack = stack->prev)
+ 				{
+ 					if (stack->source == PGC_S_FILE)
+ 					{
+ 						stack->source = PGC_S_DEFAULT;
+ 					}
+ 				}
+ 
+ 				ereport(elevel,
+ 						(errcode(ERRCODE_SUCCESSFUL_COMPLETION),
+ 						errmsg("Configuration option %s has been revert to the boot value.", gconf->name)));
+ 			}
+ 		}
+ 		gconf->status &= ~GUC_JUST_RELOAD;
+ 	}
+ 
+ 	/* Revert to environment variable. PGPORT is ignored, because it cannot be 
+ 	 * set in running state. 
+ 	 */ 
+ 	env = getenv("PGDATESTYLE");
+ 	if (env != NULL)
+ 	{
+ 		set_config_option("datestyle", env, context, 
+ 							PGC_S_ENV_VAR, false, true);
+ 	}
+ 
+ 	env = getenv("PGCLIENTENCODING");
+ 	if (env != NULL)
+ 	{
+ 		set_config_option("client_encoding", env, context, 
+ 							PGC_S_ENV_VAR, false, true);
+ 	}
+ 	}
   cleanup_list:
  	free_name_value_list(head);
  }
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.319
diff -c -r1.319 guc.c
*** src/backend/utils/misc/guc.c	11 May 2006 19:15:35 -0000	1.319
--- src/backend/utils/misc/guc.c	1 Jun 2006 14:46:56 -0000
***************
*** 2648,2686 ****
  					struct config_bool *conf = (struct config_bool *) gconf;
  
  					if (conf->assign_hook)
! 						if (!(*conf->assign_hook) (conf->reset_val, true,
  												   PGC_S_DEFAULT))
  							elog(FATAL, "failed to initialize %s to %d",
! 								 conf->gen.name, (int) conf->reset_val);
! 					*conf->variable = conf->reset_val;
  					break;
  				}
  			case PGC_INT:
  				{
  					struct config_int *conf = (struct config_int *) gconf;
  
! 					Assert(conf->reset_val >= conf->min);
! 					Assert(conf->reset_val <= conf->max);
  					if (conf->assign_hook)
! 						if (!(*conf->assign_hook) (conf->reset_val, true,
  												   PGC_S_DEFAULT))
  							elog(FATAL, "failed to initialize %s to %d",
! 								 conf->gen.name, conf->reset_val);
! 					*conf->variable = conf->reset_val;
  					break;
  				}
  			case PGC_REAL:
  				{
  					struct config_real *conf = (struct config_real *) gconf;
  
! 					Assert(conf->reset_val >= conf->min);
! 					Assert(conf->reset_val <= conf->max);
  					if (conf->assign_hook)
! 						if (!(*conf->assign_hook) (conf->reset_val, true,
  												   PGC_S_DEFAULT))
  							elog(FATAL, "failed to initialize %s to %g",
! 								 conf->gen.name, conf->reset_val);
! 					*conf->variable = conf->reset_val;
  					break;
  				}
  			case PGC_STRING:
--- 2648,2686 ----
  					struct config_bool *conf = (struct config_bool *) gconf;
  
  					if (conf->assign_hook)
! 						if (!(*conf->assign_hook) (conf->boot_val, true,
  												   PGC_S_DEFAULT))
  							elog(FATAL, "failed to initialize %s to %d",
! 								 conf->gen.name, (int) conf->boot_val);
! 					*conf->variable = conf->reset_val = conf->boot_val;
  					break;
  				}
  			case PGC_INT:
  				{
  					struct config_int *conf = (struct config_int *) gconf;
  
! 					Assert(conf->boot_val >= conf->min);
! 					Assert(conf->boot_val <= conf->max);
  					if (conf->assign_hook)
! 						if (!(*conf->assign_hook) (conf->boot_val, true,
  												   PGC_S_DEFAULT))
  							elog(FATAL, "failed to initialize %s to %d",
! 								 conf->gen.name, conf->boot_val);
! 					*conf->variable = conf->reset_val = conf->boot_val; 
  					break;
  				}
  			case PGC_REAL:
  				{
  					struct config_real *conf = (struct config_real *) gconf;
  
! 					Assert(conf->boot_val >= conf->min);
! 					Assert(conf->boot_val <= conf->max);
  					if (conf->assign_hook)
! 						if (!(*conf->assign_hook) (conf->boot_val, true,
  												   PGC_S_DEFAULT))
  							elog(FATAL, "failed to initialize %s to %g",
! 								 conf->gen.name, conf->boot_val);
! 					*conf->variable = conf->reset_val = conf->boot_val; 
  					break;
  				}
  			case PGC_STRING:
***************
*** 3133,3139 ****
  	for (i = 0; i < num_guc_variables; i++)
  	{
  		struct config_generic *gconf = guc_variables[i];
! 		int			my_status = gconf->status;
  		GucStack   *stack = gconf->stack;
  		bool		useTentative;
  		bool		changed;
--- 3133,3139 ----
  	for (i = 0; i < num_guc_variables; i++)
  	{
  		struct config_generic *gconf = guc_variables[i];
! 		int			my_status = gconf->status & (~GUC_JUST_RELOAD);
  		GucStack   *stack = gconf->stack;
  		bool		useTentative;
  		bool		changed;
***************
*** 3647,3658 ****
  		case PGC_POSTMASTER:
  			if (context == PGC_SIGHUP)
  			{
! 				if (changeVal && !is_newvalue_equal(record, value))
  					ereport(elevel,
  							(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
  							 errmsg("parameter \"%s\" cannot be changed after server start; configuration file change ignored",
  									name)));
! 
  				return true;
  			}
  			if (context != PGC_POSTMASTER)
--- 3647,3658 ----
  		case PGC_POSTMASTER:
  			if (context == PGC_SIGHUP)
  			{
! 				if (changeVal && value != NULL && !is_newvalue_equal(record, value))
  					ereport(elevel,
  							(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
  							 errmsg("parameter \"%s\" cannot be changed after server start; configuration file change ignored",
  									name)));
! 				record->status |= GUC_JUST_RELOAD; 
  				return true;
  			}
  			if (context != PGC_POSTMASTER)
***************
*** 3693,3699 ****
--- 3693,3702 ----
  				 * backend start.
  				 */
  				if (IsUnderPostmaster)
+ 				{
+ 					record->status |= GUC_JUST_RELOAD;
  					return true;
+ 				}
  			}
  			else if (context != PGC_BACKEND && context != PGC_POSTMASTER)
  			{
***************
*** 3723,3729 ****
  	 * Should we set reset/stacked values?	(If so, the behavior is not
  	 * transactional.)
  	 */
! 	makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL);
  
  	/*
  	 * Ignore attempted set if overridden by previously processed setting.
--- 3726,3732 ----
  	 * Should we set reset/stacked values?	(If so, the behavior is not
  	 * transactional.)
  	 */
! 	makeDefault = changeVal && (source <= PGC_S_OVERRIDE) && (value != NULL || source == PGC_S_FILE);
  
  	/*
  	 * Ignore attempted set if overridden by previously processed setting.
***************
*** 3766,3773 ****
  				}
  				else
  				{
! 					newval = conf->reset_val;
! 					source = conf->gen.reset_source;
  				}
  
  				if (conf->assign_hook)
--- 3769,3787 ----
  				}
  				else
  				{
! 					/* Revert value to default if source is configuration file. It is used when 
! 					 * configuration parameter is removed/commented out in the config file. Else
! 					 * RESET or SET TO DEFAULT command is called and reset_val is used. 
! 					 */
! 					if( source == PGC_S_FILE )
! 					{
! 						newval =  conf->boot_val;
! 					}
! 					else
! 					{
! 						newval = conf->reset_val;
! 						source = conf->gen.reset_source;
! 					}
  				}
  
  				if (conf->assign_hook)
***************
*** 3850,3857 ****
  				}
  				else
  				{
! 					newval = conf->reset_val;
! 					source = conf->gen.reset_source;
  				}
  
  				if (conf->assign_hook)
--- 3864,3882 ----
  				}
  				else
  				{
! 					/* Revert value to default if source is configuration file. It is used when 
! 					 * configuration parameter is removed/commented out in the config file. Else
! 					 * RESET or SET TO DEFAULT command is called and reset_val is used. 
! 					 */
! 					if( source == PGC_S_FILE )
! 					{
! 						newval =  conf->boot_val;
! 					}
! 					else
! 					{
! 						newval = conf->reset_val;
! 						source = conf->gen.reset_source;
! 					}
  				}
  
  				if (conf->assign_hook)
***************
*** 3934,3941 ****
  				}
  				else
  				{
! 					newval = conf->reset_val;
! 					source = conf->gen.reset_source;
  				}
  
  				if (conf->assign_hook)
--- 3959,3977 ----
  				}
  				else
  				{
! 					/* Revert value to default if source is configuration file. It is used when 
! 					 * configuration parameter is removed/commented out in the config file. Else
! 					 * RESET or SET TO DEFAULT command is called and reset_val is used. 
! 					 */
! 					if( source == PGC_S_FILE )
! 					{
! 						newval =  conf->boot_val;
! 					}
! 					else
! 					{
! 						newval = conf->reset_val;
! 						source = conf->gen.reset_source;
! 					}
  				}
  
  				if (conf->assign_hook)
***************
*** 4009,4014 ****
--- 4045,4064 ----
  					if (conf->gen.flags & GUC_IS_NAME)
  						truncate_identifier(newval, strlen(newval), true);
  				}
+ 				else if (source == PGC_S_FILE)
+ 				{
+ 					/* Revert value to default when item is removed from config file. */
+ 					if ( conf->boot_val != NULL )
+ 					{
+ 						newval = guc_strdup(elevel, conf->boot_val);
+ 						if (newval == NULL)
+ 							return false;
+ 					}
+ 					else
+ 					{
+ 						return false;
+ 					}
+ 				} 
  				else if (conf->reset_val)
  				{
  					/*
***************
*** 4117,4122 ****
--- 4167,4175 ----
  	if (changeVal && (record->flags & GUC_REPORT))
  		ReportGUCOption(record);
  
+ 	if(changeVal && record->source == PGC_S_FILE && value != NULL)
+ 		record->status |= GUC_JUST_RELOAD;
+ 
  	return true;
  }
  
***************
*** 5111,5116 ****
--- 5164,5174 ----
  static bool
  is_newvalue_equal(struct config_generic *record, const char *newvalue)
  {
+ 	if( !newvalue )
+ 	{
+ 		return false;
+ 	}
+   
  	switch (record->vartype)
  	{
  		case PGC_BOOL:
Index: src/include/utils/guc_tables.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/guc_tables.h,v
retrieving revision 1.22
diff -c -r1.22 guc_tables.h
*** src/include/utils/guc_tables.h	5 Mar 2006 15:59:07 -0000	1.22
--- src/include/utils/guc_tables.h	1 Jun 2006 14:46:57 -0000
***************
*** 132,138 ****
  #define GUC_HAVE_TENTATIVE	0x0001		/* tentative value is defined */
  #define GUC_HAVE_LOCAL		0x0002		/* a SET LOCAL has been executed */
  #define GUC_HAVE_STACK		0x0004		/* we have stacked prior value(s) */
! 
  
  /* GUC records for specific variable types */
  
--- 132,138 ----
  #define GUC_HAVE_TENTATIVE	0x0001		/* tentative value is defined */
  #define GUC_HAVE_LOCAL		0x0002		/* a SET LOCAL has been executed */
  #define GUC_HAVE_STACK		0x0004		/* we have stacked prior value(s) */
! #define GUC_JUST_RELOAD     0x0008      /* value is just reload from configuration file */
  
  /* GUC records for specific variable types */
  
***************
*** 142,152 ****
  	/* these fields must be set correctly in initial value: */
  	/* (all but reset_val are constants) */
  	bool	   *variable;
! 	bool		reset_val;
  	GucBoolAssignHook assign_hook;
  	GucShowHook show_hook;
  	/* variable fields, initialized at runtime: */
  	bool		tentative_val;
  };
  
  struct config_int
--- 142,153 ----
  	/* these fields must be set correctly in initial value: */
  	/* (all but reset_val are constants) */
  	bool	   *variable;
! 	bool		boot_val;
  	GucBoolAssignHook assign_hook;
  	GucShowHook show_hook;
  	/* variable fields, initialized at runtime: */
  	bool		tentative_val;
+ 	bool		reset_val;
  };
  
  struct config_int
***************
*** 155,167 ****
  	/* these fields must be set correctly in initial value: */
  	/* (all but reset_val are constants) */
  	int		   *variable;
! 	int			reset_val;
  	int			min;
  	int			max;
  	GucIntAssignHook assign_hook;
  	GucShowHook show_hook;
  	/* variable fields, initialized at runtime: */
  	int			tentative_val;
  };
  
  struct config_real
--- 156,169 ----
  	/* these fields must be set correctly in initial value: */
  	/* (all but reset_val are constants) */
  	int		   *variable;
! 	int			boot_val;
  	int			min;
  	int			max;
  	GucIntAssignHook assign_hook;
  	GucShowHook show_hook;
  	/* variable fields, initialized at runtime: */
  	int			tentative_val;
+ 	int			reset_val;
  };
  
  struct config_real
***************
*** 170,182 ****
  	/* these fields must be set correctly in initial value: */
  	/* (all but reset_val are constants) */
  	double	   *variable;
! 	double		reset_val;
  	double		min;
  	double		max;
  	GucRealAssignHook assign_hook;
  	GucShowHook show_hook;
  	/* variable fields, initialized at runtime: */
  	double		tentative_val;
  };
  
  struct config_string
--- 172,186 ----
  	/* these fields must be set correctly in initial value: */
  	/* (all but reset_val are constants) */
  	double	   *variable;
! 	double		boot_val;
  	double		min;
  	double		max;
  	GucRealAssignHook assign_hook;
  	GucShowHook show_hook;
  	/* variable fields, initialized at runtime: */
  	double		tentative_val;
+   	double		reset_val;
+  
  };
  
  struct config_string
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Reply via email to