Fix free space map to correctly track the total amount of FSM space needed

even when a single relation requires more than max_fsm_pages pages.  Also,
make VACUUM emit a warning in this case, since it likely means that VACUUM
FULL or other drastic corrective measure is needed.  Per reports from Jeff
Frost and others of unexpected changes in the claimed max_fsm_pages need.

1 files changed, 87 insertions, 34 deletions

diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index 62e5252aac1..3309ba11dac 100644
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/freespace/freespace.c,v 1.54 2006/07/14 14:52:22 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/freespace/freespace.c,v 1.55 2006/09/21 20:31:22 tgl Exp $
  *
  *
  * NOTES:
@@ -24,7 +24,7 @@
  * behavior keeps track of which relation is least recently used.
  *
  * For each known relation, we track the average request size given to
- * GetPageWithFreeSpace() as well as the most recent number of pages given
+ * GetPageWithFreeSpace() as well as the most recent number of pages reported
  * to RecordRelationFreeSpace().  The average request size is not directly
  * used in this module, but we expect VACUUM to use it to filter out
  * uninteresting amounts of space before calling RecordRelationFreeSpace().
@@ -82,7 +82,7 @@
  *		relfilenode
  *		isIndex
  *		avgRequest
- *		lastPageCount
+ *		interestingPages
  *		storedPages
  *		arena data		array of storedPages FSMPageData or IndexFSMPageData
  *----------
@@ -111,7 +111,7 @@ typedef struct FsmCacheRelHeader
 	RelFileNode key;			/* hash key (must be first) */
 	bool		isIndex;		/* if true, we store only page numbers */
 	uint32		avgRequest;		/* moving average of space requests */
-	int32		lastPageCount;	/* pages passed to RecordRelationFreeSpace */
+	BlockNumber	interestingPages;	/* # of pages with useful free space */
 	int32		storedPages;	/* # of pages stored in arena */
 } FsmCacheRelHeader;
 
@@ -128,7 +128,8 @@ static void CheckFreeSpaceMapStatistics(int elevel, int numRels,
 static FSMRelation *lookup_fsm_rel(RelFileNode *rel);
 static FSMRelation *create_fsm_rel(RelFileNode *rel);
 static void delete_fsm_rel(FSMRelation *fsmrel);
-static int	realloc_fsm_rel(FSMRelation *fsmrel, int nPages, bool isIndex);
+static int	realloc_fsm_rel(FSMRelation *fsmrel, BlockNumber interestingPages,
+							bool isIndex);
 static void link_fsm_rel_usage(FSMRelation *fsmrel);
 static void unlink_fsm_rel_usage(FSMRelation *fsmrel);
 static void link_fsm_rel_storage(FSMRelation *fsmrel);
@@ -146,6 +147,7 @@ static void pack_incoming_pages(FSMPageData *newLocation, int newPages,
 static void pack_existing_pages(FSMPageData *newLocation, int newPages,
 					FSMPageData *oldLocation, int oldPages);
 static int	fsm_calc_request(FSMRelation *fsmrel);
+static int	fsm_calc_request_unclamped(FSMRelation *fsmrel);
 static int	fsm_calc_target_allocation(int myRequest);
 static int	fsm_current_chunks(FSMRelation *fsmrel);
 static int	fsm_current_allocation(FSMRelation *fsmrel);
@@ -361,11 +363,17 @@ GetAvgFSMRequestSize(RelFileNode *rel)
  *
  * Any pre-existing info about the relation is assumed obsolete and discarded.
  *
+ * interestingPages is the total number of pages in the relation that have
+ * at least threshold free space; nPages is the number actually reported in
+ * pageSpaces[] (may be less --- in particular, callers typically clamp their
+ * space usage to MaxFSMPages).
+ *
  * The given pageSpaces[] array must be sorted in order by blkno.  Note that
  * the FSM is at liberty to discard some or all of the data.
  */
 void
 RecordRelationFreeSpace(RelFileNode *rel,
+						BlockNumber interestingPages,
 						int nPages,
 						PageFreeSpaceInfo *pageSpaces)
 {
@@ -392,7 +400,7 @@ RecordRelationFreeSpace(RelFileNode *rel,
 		int			curAllocPages;
 		FSMPageData *newLocation;
 
-		curAlloc = realloc_fsm_rel(fsmrel, nPages, false);
+		curAlloc = realloc_fsm_rel(fsmrel, interestingPages, false);
 		curAllocPages = curAlloc * CHUNKPAGES;
 
 		/*
@@ -455,6 +463,7 @@ GetFreeIndexPage(RelFileNode *rel)
  */
 void
 RecordIndexFreeSpace(RelFileNode *rel,
+					 BlockNumber interestingPages,
 					 int nPages,
 					 BlockNumber *pages)
 {
@@ -481,7 +490,7 @@ RecordIndexFreeSpace(RelFileNode *rel,
 		int			i;
 		IndexFSMPageData *newLocation;
 
-		curAlloc = realloc_fsm_rel(fsmrel, nPages, true);
+		curAlloc = realloc_fsm_rel(fsmrel, interestingPages, true);
 		curAllocPages = curAlloc * INDEXCHUNKPAGES;
 
 		/*
@@ -530,7 +539,7 @@ FreeSpaceMapTruncateRel(RelFileNode *rel, BlockNumber nblocks)
 		(void) lookup_fsm_page_entry(fsmrel, nblocks, &pageIndex);
 		/* Delete all such entries */
 		fsmrel->storedPages = pageIndex;
-		/* XXX should we adjust rel's lastPageCount and sumRequests? */
+		/* XXX should we adjust rel's interestingPages and sumRequests? */
 	}
 	LWLockRelease(FreeSpaceLock);
 }
@@ -587,20 +596,24 @@ PrintFreeSpaceMapStatistics(int elevel)
 {
 	FSMRelation *fsmrel;
 	int			storedPages = 0;
+	double		sumRequests = 0;
 	int			numRels;
-	double		sumRequests;
 	double		needed;
 
 	LWLockAcquire(FreeSpaceLock, LW_EXCLUSIVE);
-	/* Count total space used --- tedious, but seems useful */
+	/*
+	 * Count total space actually used, as well as the unclamped request total
+	 */
 	for (fsmrel = FreeSpaceMap->firstRel;
 		 fsmrel != NULL;
 		 fsmrel = fsmrel->nextPhysical)
+	{
 		storedPages += fsmrel->storedPages;
+		sumRequests += fsm_calc_request_unclamped(fsmrel);
+	}
 
 	/* Copy other stats before dropping lock */
 	numRels = FreeSpaceMap->numRels;
-	sumRequests = FreeSpaceMap->sumRequests;
 	LWLockRelease(FreeSpaceLock);
 
 	/* Convert stats to actual number of page slots needed */
@@ -613,7 +626,8 @@ PrintFreeSpaceMapStatistics(int elevel)
 			  "%.0f page slots are required to track all free space.\n"
 		  "Current limits are:  %d page slots, %d relations, using %.0f KB.",
 			  Min(needed, MaxFSMPages),
-			  needed, MaxFSMPages, MaxFSMRelations,
+			  needed,
+			  MaxFSMPages, MaxFSMRelations,
 			  (double) FreeSpaceShmemSize() / 1024.0)));
 
 	CheckFreeSpaceMapStatistics(NOTICE, numRels, needed);
@@ -687,7 +701,7 @@ DumpFreeSpaceMap(int code, Datum arg)
 		relheader.key = fsmrel->key;
 		relheader.isIndex = fsmrel->isIndex;
 		relheader.avgRequest = fsmrel->avgRequest;
-		relheader.lastPageCount = fsmrel->lastPageCount;
+		relheader.interestingPages = fsmrel->interestingPages;
 		relheader.storedPages = fsmrel->storedPages;
 		if (fwrite(&relheader, 1, sizeof(relheader), fp) != sizeof(relheader))
 			goto write_failed;
@@ -792,17 +806,12 @@ LoadFreeSpaceMap(void)
 		if (fread(&relheader, 1, sizeof(relheader), fp) != sizeof(relheader) ||
 			(relheader.isIndex != false && relheader.isIndex != true) ||
 			relheader.avgRequest >= BLCKSZ ||
-			relheader.lastPageCount < 0 ||
 			relheader.storedPages < 0)
 		{
 			elog(LOG, "bogus rel header in \"%s\"", FSM_CACHE_FILENAME);
 			goto read_failed;
 		}
 
-		/* Make sure lastPageCount doesn't exceed current MaxFSMPages */
-		if (relheader.lastPageCount > MaxFSMPages)
-			relheader.lastPageCount = MaxFSMPages;
-
 		/* Read the per-page data */
 		nPages = relheader.storedPages;
 		if (relheader.isIndex)
@@ -827,7 +836,7 @@ LoadFreeSpaceMap(void)
 		fsmrel = create_fsm_rel(&relheader.key);
 		fsmrel->avgRequest = relheader.avgRequest;
 
-		curAlloc = realloc_fsm_rel(fsmrel, relheader.lastPageCount,
+		curAlloc = realloc_fsm_rel(fsmrel, relheader.interestingPages,
 								   relheader.isIndex);
 		if (relheader.isIndex)
 		{
@@ -932,7 +941,7 @@ create_fsm_rel(RelFileNode *rel)
 		/* New hashtable entry, initialize it (hash_search set the key) */
 		fsmrel->isIndex = false;	/* until we learn different */
 		fsmrel->avgRequest = INITIAL_AVERAGE;
-		fsmrel->lastPageCount = 0;
+		fsmrel->interestingPages = 0;
 		fsmrel->firstChunk = -1;	/* no space allocated */
 		fsmrel->storedPages = 0;
 		fsmrel->nextPage = 0;
@@ -988,7 +997,8 @@ delete_fsm_rel(FSMRelation *fsmrel)
  * The return value is the actual new allocation, in chunks.
  */
 static int
-realloc_fsm_rel(FSMRelation *fsmrel, int nPages, bool isIndex)
+realloc_fsm_rel(FSMRelation *fsmrel, BlockNumber interestingPages,
+				bool isIndex)
 {
 	int			myRequest;
 	int			myAlloc;
@@ -999,7 +1009,7 @@ realloc_fsm_rel(FSMRelation *fsmrel, int nPages, bool isIndex)
 	 */
 	fsmrel->storedPages = 0;
 	FreeSpaceMap->sumRequests -= fsm_calc_request(fsmrel);
-	fsmrel->lastPageCount = nPages;
+	fsmrel->interestingPages = interestingPages;
 	fsmrel->isIndex = isIndex;
 	myRequest = fsm_calc_request(fsmrel);
 	FreeSpaceMap->sumRequests += myRequest;
@@ -1012,7 +1022,7 @@ realloc_fsm_rel(FSMRelation *fsmrel, int nPages, bool isIndex)
 	 * new data in-place.
 	 */
 	curAlloc = fsm_current_allocation(fsmrel);
-	if (myAlloc > curAlloc && (myRequest + 1) > curAlloc && nPages > 0)
+	if (myAlloc > curAlloc && (myRequest + 1) > curAlloc && interestingPages > 0)
 	{
 		/* Remove entry from storage list, and compact */
 		unlink_fsm_rel_storage(fsmrel);
@@ -1649,27 +1659,70 @@ pack_existing_pages(FSMPageData *newLocation, int newPages,
 }
 
 /*
- * Calculate number of chunks "requested" by a rel.
+ * Calculate number of chunks "requested" by a rel.  The "request" is
+ * anything beyond the rel's one guaranteed chunk.
  *
- * Rel's lastPageCount and isIndex settings must be up-to-date when called.
+ * Rel's interestingPages and isIndex settings must be up-to-date when called.
  *
  * See notes at top of file for details.
  */
 static int
 fsm_calc_request(FSMRelation *fsmrel)
 {
-	int			chunkCount;
+	int			req;
 
 	/* Convert page count to chunk count */
 	if (fsmrel->isIndex)
-		chunkCount = (fsmrel->lastPageCount - 1) / INDEXCHUNKPAGES + 1;
+	{
+		/* test to avoid unsigned underflow at zero */
+		if (fsmrel->interestingPages <= INDEXCHUNKPAGES)
+			return 0;
+		/* quotient will fit in int, even if interestingPages doesn't */
+		req = (fsmrel->interestingPages - 1) / INDEXCHUNKPAGES;
+	}
 	else
-		chunkCount = (fsmrel->lastPageCount - 1) / CHUNKPAGES + 1;
-	/* "Request" is anything beyond our one guaranteed chunk */
-	if (chunkCount <= 0)
-		return 0;
+	{
+		if (fsmrel->interestingPages <= CHUNKPAGES)
+			return 0;
+		req = (fsmrel->interestingPages - 1) / CHUNKPAGES;
+	}
+
+	/*
+	 * We clamp the per-relation requests to at most half the arena size;
+	 * this is intended to prevent a single bloated relation from crowding
+	 * out FSM service for every other rel.
+	 */
+	req = Min(req, FreeSpaceMap->totalChunks / 2);
+
+	return req;
+}
+
+/*
+ * Same as above, but without the clamp ... this is just intended for
+ * reporting the total space needed to store all information.
+ */
+static int
+fsm_calc_request_unclamped(FSMRelation *fsmrel)
+{
+	int			req;
+
+	/* Convert page count to chunk count */
+	if (fsmrel->isIndex)
+	{
+		/* test to avoid unsigned underflow at zero */
+		if (fsmrel->interestingPages <= INDEXCHUNKPAGES)
+			return 0;
+		/* quotient will fit in int, even if interestingPages doesn't */
+		req = (fsmrel->interestingPages - 1) / INDEXCHUNKPAGES;
+	}
 	else
-		return chunkCount - 1;
+	{
+		if (fsmrel->interestingPages <= CHUNKPAGES)
+			return 0;
+		req = (fsmrel->interestingPages - 1) / CHUNKPAGES;
+	}
+
+	return req;
 }
 
 /*
@@ -1769,11 +1822,11 @@ DumpFreeSpace(void)
 	for (fsmrel = FreeSpaceMap->usageList; fsmrel; fsmrel = fsmrel->nextUsage)
 	{
 		relNum++;
-		fprintf(stderr, "Map %d: rel %u/%u/%u isIndex %d avgRequest %u lastPageCount %d nextPage %d\nMap= ",
+		fprintf(stderr, "Map %d: rel %u/%u/%u isIndex %d avgRequest %u interestingPages %u nextPage %d\nMap= ",
 				relNum,
 				fsmrel->key.spcNode, fsmrel->key.dbNode, fsmrel->key.relNode,
 				(int) fsmrel->isIndex, fsmrel->avgRequest,
-				fsmrel->lastPageCount, fsmrel->nextPage);
+				fsmrel->interestingPages, fsmrel->nextPage);
 		if (fsmrel->isIndex)
 		{
 			IndexFSMPageData *page;