Re: stats collector causes shared-memory-block leakage

Tom Lane wrote:

> While investigating Scott Goodwin's recent report of trouble on Mac OS
> X, I have realized that we have an unpleasant little misbehavior in our
> last several releases. After a backend crash, the postmaster will
> attempt to recycle (delete and recreate) the old shared memory segment.
> However, if the stats collector is enabled, the two stats collection
> subprocesses are still attached to the old segment. Which means it
> doesn't go away. Instead the postmaster will set up shop with a new
> segment.
>
> This is not so bad in a system with large SHMMAX ... but if the SHMMAX
> setting is too tight to permit a second shared memory segment to be
> created, the postmaster fails.
>
> The attached patch fixes the problem by causing the stats collector to
> detach from shared memory, which it isn't using anyway. Unless I hear
> objections I will apply it to 7.4 and HEAD ... and I'm seriously
> thinking of applying it to the 7.3 branch as well.

I seem to recall there once was a pipe from the postmaster to the stat's
processes and closing that will actually get rid of them. I also seem to
recall that this was changed someday, but I don't remember why.

Anyhow, good catch. The judgement to apply to both looks right to me.

Jan

>
> regards, tom lane
>
>
> *** src/backend/port/sysv_shmem.c.orig Mon Oct 27 13:58:00 2003
> --- src/backend/port/sysv_shmem.c Thu Nov 6 18:10:02 2003
> ***************
> *** 253,258 ****
> --- 253,261 ----
> return hdr;
> }
>
> + /* Make sure PGSharedMemoryAttach doesn't fail without need */
> + UsedShmemSegAddr = NULL;
> +
> /* Loop till we find a free IPC key */
> NextShmemSegID = port * 1000;
>
> ***************
> *** 326,341 ****
> hdr->totalsize = size;
> hdr->freeoffset = MAXALIGN(sizeof(PGShmemHeader));
>
> !
> ! if (ExecBackend && UsedShmemSegAddr == NULL && !makePrivate)
> ! {
> ! UsedShmemSegAddr = memAddress;
> ! UsedShmemSegID = NextShmemSegID;
> ! }
>
> return hdr;
> }
>
>
> /*
> * Attach to shared memory and make sure it has a Postgres header
> --- 329,360 ----
> hdr->totalsize = size;
> hdr->freeoffset = MAXALIGN(sizeof(PGShmemHeader));
>
> ! /* Save info for possible future use */
> ! UsedShmemSegAddr = memAddress;
> ! UsedShmemSegID = NextShmemSegID;
>
> return hdr;
> }
>
> + /*
> + * PGSharedMemoryDetach
> + *
> + * Detach from the shared memory segment, if still attached. This is not
> + * intended for use by the process that originally created the segment
> + * (it will have an on_shmem_exit callback registered to do that). Rather,
> + * this is for subprocesses that have inherited an attachment and want to
> + * get rid of it.
> + */
> + void
> + PGSharedMemoryDetach(void)
> + {
> + if (UsedShmemSegAddr != NULL)
> + {
> + if (shmdt(UsedShmemSegAddr) < 0)
> + elog(LOG, "shmdt(%p) failed: %m", UsedShmemSegAddr);
> + UsedShmemSegAddr = NULL;
> + }
> + }
>
> /*
> * Attach to shared memory and make sure it has a Postgres header
> *** src/backend/postmaster/pgstat.c.orig Thu Sep 25 10:23:09 2003
> --- src/backend/postmaster/pgstat.c Thu Nov 6 18:10:46 2003
> ***************
> *** 44,49 ****
> --- 44,50 ----
> #include "utils/memutils.h"
> #include "storage/backendid.h"
> #include "storage/ipc.h"
> + #include "storage/pg_shmem.h"
> #include "utils/rel.h"
> #include "utils/hsearch.h"
> #include "utils/ps_status.h"
> ***************
> *** 399,404 ****
> --- 400,408 ----
>
> /* Close the postmaster's sockets, except for pgstat link */
> ClosePostmasterPorts(false);
> +
> + /* Drop our connection to postmaster's shared memory, as well */
> + PGSharedMemoryDetach();
>
> pgstat_main();
>
> *** src/include/storage/pg_shmem.h.orig Sun Aug 3 23:01:43 2003
> --- src/include/storage/pg_shmem.h Thu Nov 6 18:09:50 2003
> ***************
> *** 44,48 ****
> --- 44,49 ----
> extern PGShmemHeader *PGSharedMemoryCreate(uint32 size, bool makePrivate,
> int port);
> extern bool PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2);
> + extern void PGSharedMemoryDetach(void);
>
> #endif /* PG_SHMEM_H */
>
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #