Обсуждение: 8.2 pg_freespacemap crash

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).

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

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