Re: [HACKERS] Proper cleanup at backend exit

From: Bruce Momjian <maillist(at)candle(dot)pha(dot)pa(dot)us>
To: tgl(at)sss(dot)pgh(dot)pa(dot)us (Tom Lane)
Cc: dz(at)cs(dot)unitn(dot)it, hackers(at)postgreSQL(dot)org
Subject: Re: [HACKERS] Proper cleanup at backend exit
Date: 1998-10-01 22:58:39
Message-ID: 199810012258.SAA19007@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> Massimo Dal Zotto <dz(at)cs(dot)unitn(dot)it> writes:
> >> I have fixed several contributing bugs
> >> (having to do with the limited size of the on_shmem_exit table),
>
> > could you explain this to us ?
>
> async.c tries to arrange to clean up its entries in pg_listener by
> scheduling an exit callback routine via on_shmem_exit. (I think
> Bruce or someone has been changing that stuff, and that on_shmem_exit
> used to be called something else, but it's still basically an atexit-
> like list of procedures to call.)

Yes. The old code atexit() was called for process exit and
shared-memory reset, and that caused problems as more and more cleanup
had to be done ONLY at shared memory reset time. Having two separate
queues and proc names helped.

>
> The trouble is that on_shmem_exit has a fixed-size, and not very large,
> array for the callbacks. Try to register too many exit callbacks, and
> after a while they stop getting registered.
>
> I saw two problems associated with that. First, async.c was registering
> a separate callback for each pg_listen entry it's ever made. That was
> easily dealt with, since we now have an "unlisten all" subroutine:
> I got rid of the per-listen callback and now register only a single
> callback over the life of the backend. It just calls UnlistenAll.

That is why you need some type of global variable that sets the cleanup
the first time, and only once. You could just call the proc_exit call,
and prevent it from scheduling two of the same exit functions. That
would be cleaner.

>
> Second, the main loop in postgres.c was miscoded so that every time you
> had an error, it added a redundant FlushBufferPool callback to the list.
> Make enough mistakes before you do your first listen, and you're still
> out of luck. A simple reordering of the code fixes that. (While I was
> at it I tried to improve the comments to make it clear where the main
> loop *really* starts, so that future hackers won't misplace code in
> PostgresMain the same way again...)
>
> In the long term it might be a good idea to make on_shmem_exit construct
> a linked list of registered callbacks (eg using Dllist) rather than
> having a fixed maximum number of exit callbacks. But just at the moment
> I don't think it's necessary, given these bug fixes --- and if we did
> have a linked list in there, we'd still want to make these changes
> because they would be memory leaks otherwise.

--
Bruce Momjian | http://www.op.net/~candle
maillist(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Taral 1998-10-01 23:45:33 Re: [GENERAL] Long update query ? (also Re: [GENERAL] CNF vs. DNF)
Previous Message Bruce Momjian 1998-10-01 22:54:19 Re: [GENERAL] Long update query ?