Tom Lane wrote:
> Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > I'm having a hard time reproducing this problem; I added a pg_usleep()
> > just before get_rel_name to have the chance to drop the table, but
> > strangely enough it doesn't return NULL.  It seems that the cache entry
> > is not getting invalidated.  Maybe there's something else that's needed
> > to reproduce the crash.
> 
> I think cache invals would get noticed at points where we had to open
> some relation, so you probably need the sleep somewhere earlier than
> that.  Or just throw in an AcceptInvalidationMessages() after the sleep.

AcceptInvalidationMessages did the trick.

Patches for 8.3 and HEAD attached.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/postmaster/autovacuum.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/postmaster/autovacuum.c,v
retrieving revision 1.80
diff -c -p -r1.80 autovacuum.c
*** src/backend/postmaster/autovacuum.c	1 Jul 2008 02:09:34 -0000	1.80
--- src/backend/postmaster/autovacuum.c	17 Jul 2008 17:09:42 -0000
*************** typedef struct autovac_table
*** 176,181 ****
--- 176,184 ----
  	int			at_vacuum_cost_delay;
  	int			at_vacuum_cost_limit;
  	bool		at_wraparound;
+ 	char	   *at_relname;
+ 	char	   *at_nspname;
+ 	char	   *at_datname;
  } autovac_table;
  
  /*-------------
*************** static void relation_needs_vacanalyze(Oi
*** 282,296 ****
  						  PgStat_StatTabEntry *tabentry, bool *dovacuum,
  						  bool *doanalyze, bool *wraparound);
  
! static void autovacuum_do_vac_analyze(Oid relid, bool dovacuum,
! 						  bool doanalyze, int freeze_min_age,
! 						  bool for_wraparound,
  						  BufferAccessStrategy bstrategy);
  static HeapTuple get_pg_autovacuum_tuple_relid(Relation avRel, Oid relid);
  static PgStat_StatTabEntry *get_pgstat_tabentry_relid(Oid relid, bool isshared,
  						  PgStat_StatDBEntry *shared,
  						  PgStat_StatDBEntry *dbentry);
! static void autovac_report_activity(VacuumStmt *vacstmt, Oid relid);
  static void avl_sighup_handler(SIGNAL_ARGS);
  static void avl_sigusr1_handler(SIGNAL_ARGS);
  static void avl_sigterm_handler(SIGNAL_ARGS);
--- 285,297 ----
  						  PgStat_StatTabEntry *tabentry, bool *dovacuum,
  						  bool *doanalyze, bool *wraparound);
  
! static void autovacuum_do_vac_analyze(autovac_table *tab,
  						  BufferAccessStrategy bstrategy);
  static HeapTuple get_pg_autovacuum_tuple_relid(Relation avRel, Oid relid);
  static PgStat_StatTabEntry *get_pgstat_tabentry_relid(Oid relid, bool isshared,
  						  PgStat_StatDBEntry *shared,
  						  PgStat_StatDBEntry *dbentry);
! static void autovac_report_activity(autovac_table *tab);
  static void avl_sighup_handler(SIGNAL_ARGS);
  static void avl_sigusr1_handler(SIGNAL_ARGS);
  static void avl_sigterm_handler(SIGNAL_ARGS);
*************** do_autovacuum(void)
*** 2061,2069 ****
  		autovac_table *tab;
  		WorkerInfo	worker;
  		bool		skipit;
- 		char	   *datname,
- 				   *nspname,
- 				   *relname;
  
  		CHECK_FOR_INTERRUPTS();
  
--- 2062,2067 ----
*************** do_autovacuum(void)
*** 2158,2170 ****
  
  		/*
  		 * Save the relation name for a possible error message, to avoid a
! 		 * catalog lookup in case of an error.	Note: they must live in a
! 		 * long-lived memory context because we call vacuum and analyze in
! 		 * different transactions.
! 		 */
! 		datname = get_database_name(MyDatabaseId);
! 		nspname = get_namespace_name(get_rel_namespace(tab->at_relid));
! 		relname = get_rel_name(tab->at_relid);
  
  		/*
  		 * We will abort vacuuming the current table if something errors out,
--- 2156,2172 ----
  
  		/*
  		 * Save the relation name for a possible error message, to avoid a
! 		 * catalog lookup in case of an error.  If any of these return NULL,
! 		 * then the relation has been dropped since last we checked; skip it.
! 		 * Note: they must live in a long-lived memory context because we call
! 		 * vacuum and analyze in different transactions.
! 		 */
! 
! 		tab->at_relname = get_rel_name(tab->at_relid);
! 		tab->at_nspname = get_namespace_name(get_rel_namespace(tab->at_relid));
! 		tab->at_datname = get_database_name(MyDatabaseId);
! 		if (!tab->at_relname || !tab->at_nspname || !tab->at_datname)
! 			goto deleted;
  
  		/*
  		 * We will abort vacuuming the current table if something errors out,
*************** do_autovacuum(void)
*** 2175,2186 ****
  		{
  			/* have at it */
  			MemoryContextSwitchTo(TopTransactionContext);
! 			autovacuum_do_vac_analyze(tab->at_relid,
! 									  tab->at_dovacuum,
! 									  tab->at_doanalyze,
! 									  tab->at_freeze_min_age,
! 									  tab->at_wraparound,
! 									  bstrategy);
  
  			/*
  			 * Clear a possible query-cancel signal, to avoid a late reaction
--- 2177,2183 ----
  		{
  			/* have at it */
  			MemoryContextSwitchTo(TopTransactionContext);
! 			autovacuum_do_vac_analyze(tab, bstrategy);
  
  			/*
  			 * Clear a possible query-cancel signal, to avoid a late reaction
*************** do_autovacuum(void)
*** 2199,2208 ****
  			HOLD_INTERRUPTS();
  			if (tab->at_dovacuum)
  				errcontext("automatic vacuum of table \"%s.%s.%s\"",
! 						   datname, nspname, relname);
  			else
  				errcontext("automatic analyze of table \"%s.%s.%s\"",
! 						   datname, nspname, relname);
  			EmitErrorReport();
  
  			/* this resets the PGPROC flags too */
--- 2196,2205 ----
  			HOLD_INTERRUPTS();
  			if (tab->at_dovacuum)
  				errcontext("automatic vacuum of table \"%s.%s.%s\"",
! 						   tab->at_datname, tab->at_nspname, tab->at_relname);
  			else
  				errcontext("automatic analyze of table \"%s.%s.%s\"",
! 						   tab->at_datname, tab->at_nspname, tab->at_relname);
  			EmitErrorReport();
  
  			/* this resets the PGPROC flags too */
*************** do_autovacuum(void)
*** 2219,2228 ****
  		/* the PGPROC flags are reset at the next end of transaction */
  
  		/* be tidy */
  		pfree(tab);
- 		pfree(datname);
- 		pfree(nspname);
- 		pfree(relname);
  
  		/* remove my info from shared memory */
  		LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
--- 2216,2229 ----
  		/* the PGPROC flags are reset at the next end of transaction */
  
  		/* be tidy */
+ deleted:
+ 		if (tab->at_datname != NULL)
+ 			pfree(tab->at_datname);
+ 		if (tab->at_nspname != NULL)
+ 			pfree(tab->at_nspname);
+ 		if (tab->at_relname != NULL)
+ 			pfree(tab->at_relname);
  		pfree(tab);
  
  		/* remove my info from shared memory */
  		LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
*************** get_pgstat_tabentry_relid(Oid relid, boo
*** 2299,2304 ****
--- 2300,2307 ----
   * Recheck whether a plain table still needs vacuum or analyze; be it because
   * it does directly, or because its TOAST table does.  Return value is a valid
   * autovac_table pointer if it does, NULL otherwise.
+  *
+  * Note that the returned autovac_table does not have the name fields set.
   */
  static autovac_table *
  table_recheck_autovac(Oid relid)
*************** table_recheck_autovac(Oid relid)
*** 2437,2442 ****
--- 2440,2448 ----
  		tab->at_vacuum_cost_limit = vac_cost_limit;
  		tab->at_vacuum_cost_delay = vac_cost_delay;
  		tab->at_wraparound = wraparound || toast_wraparound;
+ 		tab->at_relname = NULL;
+ 		tab->at_nspname = NULL;
+ 		tab->at_datname = NULL;
  	}
  
  	heap_close(avRel, AccessShareLock);
*************** relation_needs_vacanalyze(Oid relid,
*** 2607,2614 ****
   *		Vacuum and/or analyze the specified table
   */
  static void
! autovacuum_do_vac_analyze(Oid relid, bool dovacuum, bool doanalyze,
! 						  int freeze_min_age, bool for_wraparound,
  						  BufferAccessStrategy bstrategy)
  {
  	VacuumStmt	vacstmt;
--- 2613,2619 ----
   *		Vacuum and/or analyze the specified table
   */
  static void
! autovacuum_do_vac_analyze(autovac_table *tab,
  						  BufferAccessStrategy bstrategy)
  {
  	VacuumStmt	vacstmt;
*************** autovacuum_do_vac_analyze(Oid relid, boo
*** 2617,2634 ****
  	MemSet(&vacstmt, 0, sizeof(vacstmt));
  
  	vacstmt.type = T_VacuumStmt;
! 	vacstmt.vacuum = dovacuum;
  	vacstmt.full = false;
! 	vacstmt.analyze = doanalyze;
! 	vacstmt.freeze_min_age = freeze_min_age;
  	vacstmt.verbose = false;
  	vacstmt.relation = NULL;	/* not used since we pass a relid */
  	vacstmt.va_cols = NIL;
  
  	/* Let pgstat know what we're doing */
! 	autovac_report_activity(&vacstmt, relid);
  
! 	vacuum(&vacstmt, relid, bstrategy, for_wraparound, true);
  }
  
  /*
--- 2622,2639 ----
  	MemSet(&vacstmt, 0, sizeof(vacstmt));
  
  	vacstmt.type = T_VacuumStmt;
! 	vacstmt.vacuum = tab->at_dovacuum;
  	vacstmt.full = false;
! 	vacstmt.analyze = tab->at_doanalyze;
! 	vacstmt.freeze_min_age = tab->at_freeze_min_age;
  	vacstmt.verbose = false;
  	vacstmt.relation = NULL;	/* not used since we pass a relid */
  	vacstmt.va_cols = NIL;
  
  	/* Let pgstat know what we're doing */
! 	autovac_report_activity(tab);
  
! 	vacuum(&vacstmt, tab->at_relid, bstrategy, tab->at_wraparound, true);
  }
  
  /*
*************** autovacuum_do_vac_analyze(Oid relid, boo
*** 2643,2679 ****
   * bother to report "<IDLE>" or some such.
   */
  static void
! autovac_report_activity(VacuumStmt *vacstmt, Oid relid)
  {
- 	char	   *relname = get_rel_name(relid);
- 	char	   *nspname = get_namespace_name(get_rel_namespace(relid));
- 
  #define MAX_AUTOVAC_ACTIV_LEN (NAMEDATALEN * 2 + 32)
! 	char		activity[MAX_AUTOVAC_ACTIV_LEN];
  
  	/* Report the command and possible options */
! 	if (vacstmt->vacuum)
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
  				 "autovacuum: VACUUM%s",
! 				 vacstmt->analyze ? " ANALYZE" : "");
  	else
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
  				 "autovacuum: ANALYZE");
  
  	/*
  	 * Report the qualified name of the relation.
- 	 *
- 	 * Paranoia is appropriate here in case relation was recently dropped ---
- 	 * the lsyscache routines we just invoked will return NULL rather than
- 	 * failing.
  	 */
! 	if (relname && nspname)
! 	{
! 		int			len = strlen(activity);
  
! 		snprintf(activity + len, MAX_AUTOVAC_ACTIV_LEN - len,
! 				 " %s.%s", nspname, relname);
! 	}
  
  	/* Set statement_timestamp() to current time for pg_stat_activity */
  	SetCurrentStatementStartTimestamp();
--- 2648,2675 ----
   * bother to report "<IDLE>" or some such.
   */
  static void
! autovac_report_activity(autovac_table *tab)
  {
  #define MAX_AUTOVAC_ACTIV_LEN (NAMEDATALEN * 2 + 32)
! 	char	activity[MAX_AUTOVAC_ACTIV_LEN];
! 	int		len;
  
  	/* Report the command and possible options */
! 	if (tab->at_dovacuum)
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
  				 "autovacuum: VACUUM%s",
! 				 tab->at_doanalyze ? " ANALYZE" : "");
  	else
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
  				 "autovacuum: ANALYZE");
  
  	/*
  	 * Report the qualified name of the relation.
  	 */
! 	len = strlen(activity);
  
! 	snprintf(activity + len, MAX_AUTOVAC_ACTIV_LEN - len,
! 			 " %s.%s", tab->at_nspname, tab->at_relname);
  
  	/* Set statement_timestamp() to current time for pg_stat_activity */
  	SetCurrentStatementStartTimestamp();
Index: src/backend/postmaster/autovacuum.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/postmaster/autovacuum.c,v
retrieving revision 1.71.2.3
diff -c -p -r1.71.2.3 autovacuum.c
*** src/backend/postmaster/autovacuum.c	14 Mar 2008 23:49:33 -0000	1.71.2.3
--- src/backend/postmaster/autovacuum.c	17 Jul 2008 17:14:58 -0000
*************** do_autovacuum(void)
*** 2098,2110 ****
  
  		/*
  		 * Save the relation name for a possible error message, to avoid a
! 		 * catalog lookup in case of an error.	Note: they must live in a
! 		 * long-lived memory context because we call vacuum and analyze in
! 		 * different transactions.
  		 */
  		datname = get_database_name(MyDatabaseId);
  		nspname = get_namespace_name(get_rel_namespace(tab->at_relid));
  		relname = get_rel_name(tab->at_relid);
  
  		/*
  		 * We will abort vacuuming the current table if something errors out,
--- 2098,2113 ----
  
  		/*
  		 * Save the relation name for a possible error message, to avoid a
! 		 * catalog lookup in case of an error.  If any of these return NULL,
! 		 * then the relation has been dropped since last we checked; skip it.
! 		 * Note: they must live in a long-lived memory context because we call
! 		 * vacuum and analyze in different transactions.
  		 */
  		datname = get_database_name(MyDatabaseId);
  		nspname = get_namespace_name(get_rel_namespace(tab->at_relid));
  		relname = get_rel_name(tab->at_relid);
+ 		if (!datname || !nspname || !relname)
+ 			goto deleted;
  
  		/*
  		 * We will abort vacuuming the current table if something errors out,
*************** do_autovacuum(void)
*** 2158,2168 ****
  
  		/* the PGPROC flags are reset at the next end of transaction */
  
  		/* be tidy */
  		pfree(tab);
! 		pfree(datname);
! 		pfree(nspname);
! 		pfree(relname);
  
  		/* remove my info from shared memory */
  		LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
--- 2161,2175 ----
  
  		/* the PGPROC flags are reset at the next end of transaction */
  
+ deleted:
  		/* be tidy */
  		pfree(tab);
! 		if (datname)
! 			pfree(datname);
! 		if (nspname)
! 			pfree(nspname);
! 		if (relname)
! 			pfree(relname);
  
  		/* remove my info from shared memory */
  		LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
-- 
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