This weekend I am trying to fix up all known the pg_autovacuum issues
that should be resolved for 7.4.3.  I am aware of only two issues:  temp
table issues, and unchecked send_query() calls, if I am forgetting
something, please let me know.

1) temp table issue:  
I was not able to reproduce the crash associated with temp tables.  I
spent a while creating tables doing updates and dropping them trying
without success to get pg_autovacuum to crash.  Since I couldn't trigger
the problem, I will need someone else to test to see if I have fixed the
problem.  Anyway, I have modified the query to exclude temp tables from
the list of tables to work with.  So we should no longer be dealing with
temp tables at all which should side step any temp table related problem
we might have been having.

2) Unchecked send_query() function calls:
As best as I can tell, this is mostly a non-issue, but I went ahead
added a check to any section that did anything with the result of
send_query, so if this was an issue, it should be fixed now.  BTW, this
might have been the cause of the temp table related crash, but that is
just a guess.


Matthew O'Connor


*** ./pg_autovacuum.c.orig	2004-05-22 02:56:09.000000000 -0400
--- ./pg_autovacuum.c	2004-05-22 03:36:01.152691850 -0400
***************
*** 225,294 ****
  		 * tables to the list that are new
  		 */
  		res = send_query((char *) TABLE_STATS_QUERY, dbi);
! 		t = PQntuples(res);
! 
! 		/*
! 		 * First: use the tbl_list as the outer loop and the result set as
! 		 * the inner loop, this will determine what tables should be
! 		 * removed
! 		 */
! 		while (tbl_elem != NULL)
! 		{
! 			tbl = ((tbl_info *) DLE_VAL(tbl_elem));
! 			found_match = 0;
! 
! 			for (i = 0; i < t; i++)
! 			{					/* loop through result set looking for a
! 								 * match */
! 				if (tbl->relid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
! 				{
! 					found_match = 1;
! 					break;
! 				}
! 			}
! 			if (found_match == 0)
! 			{					/* then we didn't find this tbl_elem in
! 								 * the result set */
! 				Dlelem	   *elem_to_remove = tbl_elem;
! 
! 				tbl_elem = DLGetSucc(tbl_elem);
! 				remove_table_from_list(elem_to_remove);
! 			}
! 			else
! 				tbl_elem = DLGetSucc(tbl_elem);
! 		}						/* Done removing dropped tables from the
! 								 * table_list */
! 
! 		/*
! 		 * Then loop use result set as outer loop and tbl_list as the
! 		 * inner loop to determine what tables are new
! 		 */
! 		for (i = 0; i < t; i++)
  		{
! 			tbl_elem = DLGetHead(dbi->table_list);
! 			found_match = 0;
  			while (tbl_elem != NULL)
  			{
  				tbl = ((tbl_info *) DLE_VAL(tbl_elem));
! 				if (tbl->relid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
! 				{
! 					found_match = 1;
! 					break;
  				}
! 				tbl_elem = DLGetSucc(tbl_elem);
! 			}
! 			if (found_match == 0)		/* then we didn't find this result
! 										 * now in the tbl_list */
  			{
! 				DLAddTail(dbi->table_list, DLNewElem(init_table_info(res, i, dbi)));
! 				if (args->debug >= 1)
  				{
! 					sprintf(logbuffer, "added table: %s.%s", dbi->dbname,
! 							((tbl_info *) DLE_VAL(DLGetTail(dbi->table_list)))->table_name);
! 					log_entry(logbuffer);
  				}
! 			}
! 		}						/* end of for loop that adds tables */
  		fflush(LOGOUTPUT);
  		PQclear(res);
  		res = NULL;
--- 225,297 ----
  		 * tables to the list that are new
  		 */
  		res = send_query((char *) TABLE_STATS_QUERY, dbi);
! 		if (res != NULL)
  		{
! 			t = PQntuples(res);
! 			
! 			/*
! 			* First: use the tbl_list as the outer loop and the result set as
! 			* the inner loop, this will determine what tables should be
! 			* removed
! 			*/
  			while (tbl_elem != NULL)
  			{
  				tbl = ((tbl_info *) DLE_VAL(tbl_elem));
! 				found_match = 0;
! 				
! 				for (i = 0; i < t; i++)
! 				{					/* loop through result set looking for a
! 									* match */
! 					if (tbl->relid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
! 					{
! 						found_match = 1;
! 						break;
! 					}
  				}
! 				if (found_match == 0)
! 				{					/* then we didn't find this tbl_elem in
! 									* the result set */
! 					Dlelem	   *elem_to_remove = tbl_elem;
! 					
! 					tbl_elem = DLGetSucc(tbl_elem);
! 					remove_table_from_list(elem_to_remove);
! 				}
! 				else
! 					tbl_elem = DLGetSucc(tbl_elem);
! 			}						/* Done removing dropped tables from the
! 									* table_list */
! 			
! 			/*
! 			* Then loop use result set as outer loop and tbl_list as the
! 			* inner loop to determine what tables are new
! 			*/
! 			for (i = 0; i < t; i++)
  			{
! 				tbl_elem = DLGetHead(dbi->table_list);
! 				found_match = 0;
! 				while (tbl_elem != NULL)
  				{
! 					tbl = ((tbl_info *) DLE_VAL(tbl_elem));
! 					if (tbl->relid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
! 					{
! 						found_match = 1;
! 						break;
! 					}
! 					tbl_elem = DLGetSucc(tbl_elem);
  				}
! 				if (found_match == 0)		/* then we didn't find this result
! 											* now in the tbl_list */
! 				{
! 					DLAddTail(dbi->table_list, DLNewElem(init_table_info(res, i, dbi)));
! 					if (args->debug >= 1)
! 					{
! 						sprintf(logbuffer, "added table: %s.%s", dbi->dbname,
! 								((tbl_info *) DLE_VAL(DLGetTail(dbi->table_list)))->table_name);
! 						log_entry(logbuffer);
! 					}
! 				}
! 			}						/* end of for loop that adds tables */
! 		}
  		fflush(LOGOUTPUT);
  		PQclear(res);
  		res = NULL;
***************
*** 410,422 ****
  	if (dbs->conn != NULL)
  	{
  		res = send_query(FROZENOID_QUERY, dbs);
! 		dbs->oid = atooid(PQgetvalue(res, 0, PQfnumber(res, "oid")));
! 		dbs->age = atol(PQgetvalue(res, 0, PQfnumber(res, "age")));
! 		if (res)
! 			PQclear(res);
! 
! 		if (args->debug >= 2)
! 			print_db_list(db_list, 0);
  	}
  	return db_list;
  }
--- 413,430 ----
  	if (dbs->conn != NULL)
  	{
  		res = send_query(FROZENOID_QUERY, dbs);
! 		if (res != NULL)
! 		{
! 			dbs->oid = atooid(PQgetvalue(res, 0, PQfnumber(res, "oid")));
! 			dbs->age = atol(PQgetvalue(res, 0, PQfnumber(res, "age")));
! 			if (res)
! 				PQclear(res);
! 	
! 			if (args->debug >= 2)
! 				print_db_list(db_list, 0);
! 		}
! 		else
! 			return NULL;
  	}
  	return db_list;
  }
***************
*** 488,565 ****
  		 * add databases to the list that are new
  		 */
  		res = send_query(FROZENOID_QUERY2, dbi_template1);
! 		t = PQntuples(res);
! 
! 		/*
! 		 * First: use the db_list as the outer loop and the result set as
! 		 * the inner loop, this will determine what databases should be
! 		 * removed
! 		 */
! 		while (db_elem != NULL)
  		{
! 			dbi = ((db_info *) DLE_VAL(db_elem));
! 			found_match = 0;
! 
! 			for (i = 0; i < t; i++)
! 			{					/* loop through result set looking for a
! 								 * match */
! 				if (dbi->oid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
! 				{
! 					found_match = 1;
! 
! 					/*
! 					 * update the dbi->age so that we ensure
! 					 * xid_wraparound won't happen
! 					 */
! 					dbi->age = atol(PQgetvalue(res, i, PQfnumber(res, "age")));
! 					break;
! 				}
! 			}
! 			if (found_match == 0)
! 			{					/* then we didn't find this db_elem in the
! 								 * result set */
! 				Dlelem	   *elem_to_remove = db_elem;
! 
! 				db_elem = DLGetSucc(db_elem);
! 				remove_db_from_list(elem_to_remove);
! 			}
! 			else
! 				db_elem = DLGetSucc(db_elem);
! 		}						/* Done removing dropped databases from
! 								 * the table_list */
! 
! 		/*
! 		 * Then loop use result set as outer loop and db_list as the inner
! 		 * loop to determine what databases are new
! 		 */
! 		for (i = 0; i < t; i++)
! 		{
! 			db_elem = DLGetHead(db_list);
! 			found_match = 0;
  			while (db_elem != NULL)
  			{
  				dbi = ((db_info *) DLE_VAL(db_elem));
! 				if (dbi->oid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
! 				{
! 					found_match = 1;
! 					break;
  				}
! 				db_elem = DLGetSucc(db_elem);
! 			}
! 			if (found_match == 0)		/* then we didn't find this result
! 										 * now in the tbl_list */
  			{
! 				DLAddTail(db_list, DLNewElem(init_dbinfo
! 						  (PQgetvalue(res, i, PQfnumber(res, "datname")),
! 						 atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))),
! 					  atol(PQgetvalue(res, i, PQfnumber(res, "age"))))));
! 				if (args->debug >= 1)
  				{
! 					sprintf(logbuffer, "added database: %s", ((db_info *) DLE_VAL(DLGetTail(db_list)))->dbname);
! 					log_entry(logbuffer);
  				}
! 			}
! 		}						/* end of for loop that adds tables */
  		fflush(LOGOUTPUT);
  		PQclear(res);
  		res = NULL;
--- 496,576 ----
  		 * add databases to the list that are new
  		 */
  		res = send_query(FROZENOID_QUERY2, dbi_template1);
! 		if (res != NULL)
  		{
! 			t = PQntuples(res);
! 	
! 			/*
! 			* First: use the db_list as the outer loop and the result set as
! 			* the inner loop, this will determine what databases should be
! 			* removed
! 			*/
  			while (db_elem != NULL)
  			{
  				dbi = ((db_info *) DLE_VAL(db_elem));
! 				found_match = 0;
! 	
! 				for (i = 0; i < t; i++)
! 				{					/* loop through result set looking for a
! 									* match */
! 					if (dbi->oid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
! 					{
! 						found_match = 1;
! 	
! 						/*
! 						* update the dbi->age so that we ensure
! 						* xid_wraparound won't happen
! 						*/
! 						dbi->age = atol(PQgetvalue(res, i, PQfnumber(res, "age")));
! 						break;
! 					}
  				}
! 				if (found_match == 0)
! 				{					/* then we didn't find this db_elem in the
! 									* result set */
! 					Dlelem	   *elem_to_remove = db_elem;
! 	
! 					db_elem = DLGetSucc(db_elem);
! 					remove_db_from_list(elem_to_remove);
! 				}
! 				else
! 					db_elem = DLGetSucc(db_elem);
! 			}						/* Done removing dropped databases from
! 									* the table_list */
! 	
! 			/*
! 			* Then loop use result set as outer loop and db_list as the inner
! 			* loop to determine what databases are new
! 			*/
! 			for (i = 0; i < t; i++)
  			{
! 				db_elem = DLGetHead(db_list);
! 				found_match = 0;
! 				while (db_elem != NULL)
  				{
! 					dbi = ((db_info *) DLE_VAL(db_elem));
! 					if (dbi->oid == atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))))
! 					{
! 						found_match = 1;
! 						break;
! 					}
! 					db_elem = DLGetSucc(db_elem);
  				}
! 				if (found_match == 0)		/* then we didn't find this result
! 											* now in the tbl_list */
! 				{
! 					DLAddTail(db_list, DLNewElem(init_dbinfo
! 							(PQgetvalue(res, i, PQfnumber(res, "datname")),
! 							atooid(PQgetvalue(res, i, PQfnumber(res, "oid"))),
! 						atol(PQgetvalue(res, i, PQfnumber(res, "age"))))));
! 					if (args->debug >= 1)
! 					{
! 						sprintf(logbuffer, "added database: %s", ((db_info *) DLE_VAL(DLGetTail(db_list)))->dbname);
! 						log_entry(logbuffer);
! 					}
! 				}
! 			}						/* end of for loop that adds tables */
! 		}
  		fflush(LOGOUTPUT);
  		PQclear(res);
  		res = NULL;
***************
*** 599,605 ****
  
  		res = send_query("VACUUM", dbi);
  		/* FIXME: Perhaps should add a check for PQ_COMMAND_OK */
! 		PQclear(res);
  		return 1;
  	}
  	return 0;
--- 610,619 ----
  
  		res = send_query("VACUUM", dbi);
  		/* FIXME: Perhaps should add a check for PQ_COMMAND_OK */
! 		if (res != NULL)
! 		{
! 			PQclear(res);
! 		}
  		return 1;
  	}
  	return 0;
***************
*** 750,756 ****
  	int			ret = 0;
  
  	res = send_query("SHOW stats_row_level", dbi);
! 	if (res)
  	{
  		ret = strcmp("on", PQgetvalue(res, 0, PQfnumber(res, "stats_row_level")));
  		PQclear(res);
--- 764,770 ----
  	int			ret = 0;
  
  	res = send_query("SHOW stats_row_level", dbi);
! 	if (res != NULL)
  	{
  		ret = strcmp("on", PQgetvalue(res, 0, PQfnumber(res, "stats_row_level")));
  		PQclear(res);
***************
*** 1079,1159 ****
  					res = send_query(TABLE_STATS_QUERY, dbs);	/* Get an updated
  																 * snapshot of this dbs
  																 * table stats */
! 					for (j = 0; j < PQntuples(res); j++)
! 					{			/* loop through result set */
! 						tbl_elem = DLGetHead(dbs->table_list);	/* Reset tbl_elem to top
! 																 * of dbs->table_list */
! 						while (tbl_elem != NULL)
! 						{		/* Loop through tables in list */
! 							tbl = ((tbl_info *) DLE_VAL(tbl_elem));		/* set tbl_info =
! 																		 * current_table */
! 							if (tbl->relid == atooid(PQgetvalue(res, j, PQfnumber(res, "oid"))))
! 							{
! 								tbl->curr_analyze_count =
! 									(atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_ins"))) +
! 									 atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_upd"))) +
! 									 atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_del"))));
! 								tbl->curr_vacuum_count =
! 									(atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_del"))) +
! 									 atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_upd"))));
! 
! 								/*
! 								 * Check numDeletes to see if we need to
! 								 * vacuum, if so: Run vacuum analyze
! 								 * (adding analyze is small so we might as
! 								 * well) Update table thresholds and
! 								 * related information if numDeletes is
! 								 * not big enough for vacuum then check
! 								 * numInserts for analyze
! 								 */
! 								if (tbl->curr_vacuum_count - tbl->CountAtLastVacuum >= tbl->vacuum_threshold)
  								{
  									/*
! 									 * if relisshared = t and database !=
! 									 * template1 then only do an analyze
! 									 */
! 									if (tbl->relisshared > 0 && strcmp("template1", dbs->dbname))
! 										snprintf(buf, sizeof(buf), "ANALYZE %s", tbl->table_name);
! 									else
! 										snprintf(buf, sizeof(buf), "VACUUM ANALYZE %s", tbl->table_name);
! 									if (args->debug >= 1)
  									{
! 										sprintf(logbuffer, "Performing: %s", buf);
! 										log_entry(logbuffer);
! 										fflush(LOGOUTPUT);
  									}
! 									send_query(buf, dbs);
! 									update_table_thresholds(dbs, tbl, VACUUM_ANALYZE);
! 									if (args->debug >= 2)
! 										print_table_info(tbl);
! 								}
! 								else if (tbl->curr_analyze_count - tbl->CountAtLastAnalyze >= tbl->analyze_threshold)
! 								{
! 									snprintf(buf, sizeof(buf), "ANALYZE %s", tbl->table_name);
! 									if (args->debug >= 1)
  									{
! 										sprintf(logbuffer, "Performing: %s", buf);
! 										log_entry(logbuffer);
! 										fflush(LOGOUTPUT);
  									}
! 									send_query(buf, dbs);
! 									update_table_thresholds(dbs, tbl, ANALYZE_ONLY);
! 									if (args->debug >= 2)
! 										print_table_info(tbl);
  								}
! 
! 								break;	/* once we have found a match, no
! 										 * need to keep checking. */
! 							}
! 
! 							/*
! 							 * Advance the table pointers for the next
! 							 * loop
! 							 */
! 							tbl_elem = DLGetSucc(tbl_elem);
! 
! 						}		/* end for table while loop */
! 					}			/* end for j loop (tuples in PGresult) */
  				}				/* close of if(xid_wraparound_check()) */
  				/* Done working on this db, Clean up, then advance cur_db */
  				PQclear(res);
--- 1093,1176 ----
  					res = send_query(TABLE_STATS_QUERY, dbs);	/* Get an updated
  																 * snapshot of this dbs
  																 * table stats */
! 					if (res != NULL)
! 					{
! 						for (j = 0; j < PQntuples(res); j++)
! 						{			/* loop through result set */
! 							tbl_elem = DLGetHead(dbs->table_list);	/* Reset tbl_elem to top
! 																	* of dbs->table_list */
! 							while (tbl_elem != NULL)
! 							{		/* Loop through tables in list */
! 								tbl = ((tbl_info *) DLE_VAL(tbl_elem));		/* set tbl_info =
! 																			* current_table */
! 								if (tbl->relid == atooid(PQgetvalue(res, j, PQfnumber(res, "oid"))))
  								{
+ 									tbl->curr_analyze_count =
+ 										(atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_ins"))) +
+ 										atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_upd"))) +
+ 										atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_del"))));
+ 									tbl->curr_vacuum_count =
+ 										(atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_del"))) +
+ 										atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_upd"))));
+ 	
  									/*
! 									* Check numDeletes to see if we need to
! 									* vacuum, if so: Run vacuum analyze
! 									* (adding analyze is small so we might as
! 									* well) Update table thresholds and
! 									* related information if numDeletes is
! 									* not big enough for vacuum then check
! 									* numInserts for analyze
! 									*/
! 									if (tbl->curr_vacuum_count - tbl->CountAtLastVacuum >= tbl->vacuum_threshold)
  									{
! 										/*
! 										* if relisshared = t and database !=
! 										* template1 then only do an analyze
! 										*/
! 										if (tbl->relisshared > 0 && strcmp("template1", dbs->dbname))
! 											snprintf(buf, sizeof(buf), "ANALYZE %s", tbl->table_name);
! 										else
! 											snprintf(buf, sizeof(buf), "VACUUM ANALYZE %s", tbl->table_name);
! 										if (args->debug >= 1)
! 										{
! 											sprintf(logbuffer, "Performing: %s", buf);
! 											log_entry(logbuffer);
! 											fflush(LOGOUTPUT);
! 										}
! 										send_query(buf, dbs);
! 										update_table_thresholds(dbs, tbl, VACUUM_ANALYZE);
! 										if (args->debug >= 2)
! 											print_table_info(tbl);
  									}
! 									else if (tbl->curr_analyze_count - tbl->CountAtLastAnalyze >= tbl->analyze_threshold)
  									{
! 										snprintf(buf, sizeof(buf), "ANALYZE %s", tbl->table_name);
! 										if (args->debug >= 1)
! 										{
! 											sprintf(logbuffer, "Performing: %s", buf);
! 											log_entry(logbuffer);
! 											fflush(LOGOUTPUT);
! 										}
! 										send_query(buf, dbs);
! 										update_table_thresholds(dbs, tbl, ANALYZE_ONLY);
! 										if (args->debug >= 2)
! 											print_table_info(tbl);
  									}
! 									
! 									break;	/* once we have found a match, no
! 											* need to keep checking. */
  								}
! 								
! 								/*
! 								* Advance the table pointers for the next
! 								* loop
! 								*/
! 								tbl_elem = DLGetSucc(tbl_elem);
! 								
! 							}		/* end for table while loop */
! 						}			/* end for j loop (tuples in PGresult) */
! 					}			/* end if (res != NULL) */
  				}				/* close of if(xid_wraparound_check()) */
  				/* Done working on this db, Clean up, then advance cur_db */
  				PQclear(res);
*** ./pg_autovacuum.h.orig	2004-05-22 02:56:09.000000000 -0400
--- ./pg_autovacuum.h	2004-05-22 02:30:23.000000000 -0400
***************
*** 34,40 ****
  #define VACUUM_ANALYZE		0
  #define ANALYZE_ONLY		1
  
! #define TABLE_STATS_QUERY	"select a.oid,a.relname,a.relnamespace,a.relpages,a.relisshared,a.reltuples,b.schemaname,b.n_tup_ins,b.n_tup_upd,b.n_tup_del from pg_class a, pg_stat_all_tables b where a.oid=b.relid and a.relkind = 'r'"
  
  #define FRONTEND
  #define PAGES_QUERY "select oid,reltuples,relpages from pg_class where oid=%u"
--- 34,40 ----
  #define VACUUM_ANALYZE		0
  #define ANALYZE_ONLY		1
  
! #define TABLE_STATS_QUERY	"select a.oid,a.relname,a.relnamespace,a.relpages,a.relisshared,a.reltuples,b.schemaname,b.n_tup_ins,b.n_tup_upd,b.n_tup_del from pg_class a, pg_stat_all_tables b where a.oid=b.relid and a.relkind = 'r' and schemaname not like 'pg_temp_%'"
  
  #define FRONTEND
  #define PAGES_QUERY "select oid,reltuples,relpages from pg_class where oid=%u"
---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
      subscribe-nomail command to [EMAIL PROTECTED] so that your
      message can get through to the mailing list cleanly

Reply via email to