Thread: Win32 semaphore patch
Attached is a patch for Win32 semaphore reimplementation per discussion with Tom and Magnus. Basically it implements the interfaces defined in pg_sema.h using unnamed semaphores in Win32. Change in configure.in is needed to support this modification. Regards, Qingqing
Attachment
In PGSemaphoreLock, should't ImmediateInterruptOK be reset in all cases? It's not reset if we get the semaphore now. Though I won't claim to entirely know what that flag does ;-) (btw, I got double newlines all over) //Magnus > -----Original Message----- > From: pgsql-patches-owner@postgresql.org > [mailto:pgsql-patches-owner@postgresql.org] On Behalf Of Qingqing Zhou > Sent: Thursday, April 20, 2006 2:06 PM > To: pgsql-patches@postgresql.org > Subject: [PATCHES] Win32 semaphore patch > > Attached is a patch for Win32 semaphore reimplementation per > discussion with Tom and Magnus. Basically it implements the > interfaces defined in pg_sema.h using unnamed semaphores in > Win32. Change in configure.in is needed to support this modification. > > Regards, > Qingqing >
Qingqing Zhou <zhouqq@cs.toronto.edu> writes:
> Attached is a patch for Win32 semaphore reimplementation per discussion
> with Tom and Magnus.
The trickiest part of the sysv and posix implementations is making sure
that any kernel resources represented by the semaphores will go away at
appropriate times. There are two cases to worry about:
1. Backend crash while postmaster stays alive: postmaster will get to
execute the on_shmem_exit() callback after all the backends are gone,
and that can clean 'em up.
2. postmaster crashes: we'd like the semas to go away automatically when
the last backend goes away. If this doesn't happen, then there has to
be logic to recycle leftover semas when the postmaster is next started.
Most of the ugliness in sysv_sema.c is because it has to do "manual"
cleanup per #2.
I dunno much about Windows, and it may be that your code is already OK
on this score. I'm just pointing it out as something that has to be
considered. Some comments in the code about how this works wouldn't
be a bad idea.
regards, tom lane
> > Attached is a patch for Win32 semaphore reimplementation per > > discussion with Tom and Magnus. > > The trickiest part of the sysv and posix implementations is > making sure that any kernel resources represented by the > semaphores will go away at appropriate times. There are two > cases to worry about: > > 1. Backend crash while postmaster stays alive: postmaster > will get to execute the on_shmem_exit() callback after all > the backends are gone, and that can clean 'em up. > > 2. postmaster crashes: we'd like the semas to go away > automatically when the last backend goes away. If this > doesn't happen, then there has to be logic to recycle > leftover semas when the postmaster is next started. > > Most of the ugliness in sysv_sema.c is because it has to do "manual" > cleanup per #2. > > I dunno much about Windows, and it may be that your code is > already OK on this score. I'm just pointing it out as > something that has to be considered. Some comments in the > code about how this works wouldn't be a bad idea. For #2, yes, the semaphores will go away when the last process holding a HANDLE to it goes away. For #1, the code seems to handle that right? //Magnus
"Magnus Hagander" <mha@sollentuna.net> writes:
> For #2, yes, the semaphores will go away when the last process holding a
> HANDLE to it goes away.
Well, that raises an interesting point: exactly where in this code does
ownership of the HANDLEs get propagated to the child processes? As
written, the HANDLEs seem to belong only to the postmaster --- will the
kernel calls even work in the child processes? According to what
someone was telling me the other day, HANDLEs are process-local, so just
storing them in shared memory doesn't seem like it should work.
regards, tom lane
> > For #2, yes, the semaphores will go away when the last > process holding > > a HANDLE to it goes away. > > Well, that raises an interesting point: exactly where in this > code does ownership of the HANDLEs get propagated to the > child processes? As written, the HANDLEs seem to belong only > to the postmaster --- will the kernel calls even work in the > child processes? According to what someone was telling me > the other day, HANDLEs are process-local, so just storing > them in shared memory doesn't seem like it should work. They're inherited down. I haven't looked at the calling path, but if they're all created in the postmaster *before* the backends are forked, it's not a problem. The code specifically sets them to inheritable, and if you do that you can use them in a child process. //Magnus
"Magnus Hagander" <mha@sollentuna.net> writes:
>> Well, that raises an interesting point: exactly where in this
>> code does ownership of the HANDLEs get propagated to the
>> child processes?
> The code specifically sets them to inheritable,
Oh, I see. Seems like it should work then, but all this stuff should be
mentioned in the file header comment.
regards, tom lane
""Magnus Hagander"" <mha@sollentuna.net> wrote > > > > 1. Backend crash while postmaster stays alive: postmaster > > will get to execute the on_shmem_exit() callback after all > > the backends are gone, and that can clean 'em up. > > > > 2. postmaster crashes: we'd like the semas to go away > > automatically when the last backend goes away. If this > > doesn't happen, then there has to be logic to recycle > > leftover semas when the postmaster is next started. > > > > Most of the ugliness in sysv_sema.c is because it has to do "manual" > > cleanup per #2. > > > > For #2, yes, the semaphores will go away when the last process holding a > HANDLE to it goes away. For #1, the code seems to handle that right? > I intentionally use *unnamed* semaphores to avoid these problems -- even if the semaphores didn't go away (as Magus pointed out, if all processes can exit gracefully, this won't happen), we won't worry about them -- Creating semahpores will still succeed because there is no existent same named semaphores will bother it. Regards, Qingqing
"Qingqing Zhou" <zhouqq@cs.toronto.edu> writes:
> I intentionally use *unnamed* semaphores to avoid these problems -- even if
> the semaphores didn't go away (as Magus pointed out, if all processes can
> exit gracefully, this won't happen), we won't worry about them -- Creating
> semahpores will still succeed because there is no existent same named
> semaphores will bother it.
Except that eventually you run the kernel out of resources. We were
forced to confront that point very early when dealing with the SysV
API, because of the remarkably low resource limits it traditionally
has, but long-term resource leaks are never a good idea in any software.
Or are you designing this according to the widespread view that Windows
system uptimes are measured in small numbers of days anyway?
regards, tom lane
"Tom Lane" <tgl@sss.pgh.pa.us> wrote > "Qingqing Zhou" <zhouqq@cs.toronto.edu> writes: > > I intentionally use *unnamed* semaphores to avoid these problems -- even if > > the semaphores didn't go away (as Magus pointed out, if all processes can > > exit gracefully, this won't happen), we won't worry about them -- Creating > > semahpores will still succeed because there is no existent same named > > semaphores will bother it. > > Except that eventually you run the kernel out of resources. We were > forced to confront that point very early when dealing with the SysV > API, because of the remarkably low resource limits it traditionally > has, but long-term resource leaks are never a good idea in any software. > > Or are you designing this according to the widespread view that Windows > system uptimes are measured in small numbers of days anyway? > /* BTW: I should use "evently" instead of "gracefully" in the above sentence. */ Maybe I missed the point here: If we really run out of kernel resources, I don't think we can do much even with named semaphores - because the resource leaked may not belong to any Postgres processes and we can't clean them up. Regards, Qingqing
"Qingqing Zhou" <zhouqq@cs.toronto.edu> writes:
> Maybe I missed the point here: If we really run out of kernel resources, I
> don't think we can do much even with named semaphores - because the resource
> leaked may not belong to any Postgres processes and we can't clean them up.
It's certainly not our business to defend the kernel against misbehavior
of other applications. But IMHO it *is* our business to defend against
our own misbehavior. Postgres should not be the weakest link.
Note I am not saying there's anything wrong with the code you posted;
based on discussion to date it seems to be solid. What I am taking
issue with is the attitude you seem to have that it's not our problem
if we leak resources. It is our problem.
regards, tom lane
> > For #2, yes, the semaphores will go away when the last > process holding > > a HANDLE to it goes away. For #1, the code seems to handle > that right? > > > > I intentionally use *unnamed* semaphores to avoid these > problems -- even if the semaphores didn't go away (as Magus > pointed out, if all processes can exit gracefully, this won't > happen), we won't worry about them -- Creating semahpores > will still succeed because there is no existent same named > semaphores will bother it. Just a point - they will get automatically cleaned up even if the process doesn't exit *gracefully*, as long as it exits. Only if it's hung and won't actually exit will the handles not get cleaned up. This goes for both named and unnamed ones. //Magnus
"Tom Lane" <tgl@sss.pgh.pa.us> wrote > > Note I am not saying there's anything wrong with the code you posted; > based on discussion to date it seems to be solid. What I am taking > issue with is the attitude you seem to have that it's not our problem > if we leak resources. It is our problem. > Right, I totoally agree that we should try our best to prevent resource leakage and related problems. Regards, Qingqing
""Magnus Hagander"" <mha@sollentuna.net> wrote > > Just a point - they will get automatically cleaned up even if the > process doesn't exit *gracefully*, as long as it exits. Only if it's > hung and won't actually exit will the handles not get cleaned up. > This goes for both named and unnamed ones. > Sorry, I realized that I should not write "gracefully" and corrected it in the above thread :-). Regards, Qingqing
> > I intentionally use *unnamed* semaphores to avoid these problems -- > > even if the semaphores didn't go away (as Magus pointed out, if all > > processes can exit gracefully, this won't happen), we won't worry > > about them -- Creating semahpores will still succeed > because there is > > no existent same named semaphores will bother it. > > Except that eventually you run the kernel out of resources. > We were forced to confront that point very early when dealing > with the SysV API, because of the remarkably low resource > limits it traditionally has, but long-term resource leaks are > never a good idea in any software. Most definitly. Windows will easily handle thousands, or tends of thousands, of semaphores, but leaking them is never acceptable behaviour. But the bottom line is that the objects are automatiaclly destroyed when all processes holding open handles to them exit - which IIRC is not the case with SysV ones. And if we leak *processes* that don't exit, that's a much bigger problem. > Or are you designing this according to the widespread view > that Windows system uptimes are measured in small numbers of > days anyway? Widespread misunderstanding, I'd say. Today, a properly configured windows db server shuold normally have uptimes well exceeding half a year, often over a year. No, that's not as good as a properly configured unix box, but it's certainly not few enough days not to care about leakage. So it's definitly not something we should even consider as an excuse. (note that "properly configured" includes "properly firewalled so you don't need to apply the monthly set of patches every time") //Magnus
Patch applied, and I added a mention that anonymous semaphores were used, and are deleted on last reference. --------------------------------------------------------------------------- Qingqing Zhou wrote: > Attached is a patch for Win32 semaphore reimplementation per discussion > with Tom and Magnus. Basically it implements the interfaces defined in > pg_sema.h using unnamed semaphores in Win32. Change in configure.in is > needed to support this modification. > > Regards, > Qingqing Content-Description: [ Attachment, skipping... ] Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +