des@des.no (=?iso-8859-1?q?Dag-Erling_Sm=F8rgrav?=) writes:
> The attached patch implements new semaphore and shared memory
> mechanisms for POSIX systems.
I'm afraid we'll have to reject this out of hand:
> +bool
> +PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2)
> +{
> + /*
> + * This is never the case when using mmap(), since the segments will
> + * vanish into thin air when postmaster exits or crashes.
> + */
> + return false;
> +}
This is not acceptable in the slightest, because it offers no protection
against the situation where the old postmaster has crashed but there are
still live backends. If a new postmaster and new backends are allowed
to start in that situation, using a new shared memory segment, you
*will* have major database corruption (eg, duplicate use of transaction
IDs). We need the SysV ability to detect whether any backends are still
connected to the old shared memory segment in order to be safe against
this scenario.
The semaphore code may be functionally OK, but I'm not thrilled with the
fact that it requires two open file descriptors per semaphore, which
have to be passed down to each postmaster child process. That's a lot
of files if MaxBackends is large; not only does it constrain the number
of file slots available for fd.c to use, but you run the risk of
overflowing what an fd_set can handle, which I notice breaks this code
:-(. For comparison, the Darwin implementation needs one descriptor per
semaphore, and we have seen performance issues with that.
regards, tom lane
