8.2 pg_freespacemap crash

Started by Dimitri Fontaineabout 17 years ago3 messagesbugs
Jump to latest
#1Dimitri Fontaine
dimitri@2ndQuadrant.fr

Hi,

We've been hit by a pg_freespacemap contrib module crash (SIGSEGV), gdb showed
it was in line 162 of it's pg_freespacemap.c file.
fctx->record[i].reltablespace = fsmrel->key.spcNode;

Thanks to Andrew Gierth (irc:RhodiumToad) and after some analysis and we've
been able to understand what happened: MaxFSMPages is not the maximum number
of elements (tables and indexes) contained into the FSM memory, and the
contrib believes it is.

In fact, AFAUI this number is computed from the max_fsm_pages setting applying
to it the 6 bytes per table. When you put in there some index FSM information,
you need 4 bytes. So you can store more items than max_fsm_pages * 6 bytes.

Please find attached a "stupid" patch for the case not to happen again here, I
suppose you'll want some more refined solution to get into -core.

Regards,
--
dim

PS: of course any faulty reasoning here would be mine, and we still have the
evidences around (core).

Attachments:

13-pg_freespacemap.maxfsmpages.patchtext/x-patch; charset=UTF-8; name=13-pg_freespacemap.maxfsmpages.patchDownload+1-1
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dimitri Fontaine (#1)
Re: 8.2 pg_freespacemap crash

Dimitri Fontaine <dfontaine@hi-media.com> writes:

Thanks to Andrew Gierth (irc:RhodiumToad) and after some analysis and we've
been able to understand what happened: MaxFSMPages is not the maximum number
of elements (tables and indexes) contained into the FSM memory, and the
contrib believes it is.

Hmm. Actually the freespace.c code rounds that number up to the next
multiple of CHUNKPAGES:

/* Allocate page-storage arena */
nchunks = (MaxFSMPages - 1) / CHUNKPAGES + 1;

So your 2x patch is way overkill, but we do have an issue here.

regards, tom lane

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#2)
Re: 8.2 pg_freespacemap crash

I wrote:

Hmm. Actually the freespace.c code rounds that number up to the next
multiple of CHUNKPAGES:
/* Allocate page-storage arena */
nchunks = (MaxFSMPages - 1) / CHUNKPAGES + 1;

While that's true, on closer inspection the *real* problem is that
CHUNKPAGES is only correct for heap relations. Indexes can squeeze 50%
more pages per chunk. So you probably saw the problem because you have
lots of indexes, not because you were hard up against the roundoff
boundary.

The correct patch is

Index: pg_freespacemap.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pg_freespacemap/pg_freespacemap.c,v
retrieving revision 1.9
diff -c -r1.9 pg_freespacemap.c
*** pg_freespacemap.c	19 Oct 2006 18:32:46 -0000	1.9
--- pg_freespacemap.c	7 Apr 2009 18:05:23 -0000
***************
*** 94,99 ****
--- 94,100 ----
  	if (SRF_IS_FIRSTCALL())
  	{
  		int			i;
+ 		int			nchunks;	/* Size of freespace.c's arena. */
  		int			numPages;	/* Max possible no. of pages in map. */
  		int			nPages;		/* Mapped pages for a relation. */

***************
*** 102,108 ****
*/
FreeSpaceMap = GetFreeSpaceMap();

! numPages = MaxFSMPages;

funcctx = SRF_FIRSTCALL_INIT();

--- 103,112 ----
  		 */
  		FreeSpaceMap = GetFreeSpaceMap();

! /* this must match calculation in InitFreeSpaceMap(): */
! nchunks = (MaxFSMPages - 1) / CHUNKPAGES + 1;
! /* Worst case (lots of indexes) could have this many pages: */
! numPages = nchunks * INDEXCHUNKPAGES;

funcctx = SRF_FIRSTCALL_INIT();

regards, tom lane