Re: [PATCHES] Feature: POSIX Shared memory support, round 2

2007-02-09 Thread Chris Marcellino
That is strange, because the majority of the comments are new. Much  
of the code and comments are reused from the SysV code because, you  
know, this is an enhancement. The comments that are left serve a  
purpose.
In PGSharedMemoryCreate, this implementation avoids the need to tell  
if live backends are attached to an existing segment, since exisiting  
segments are not reattached to--the old segments are cleared when the  
live orphan backends die.
I would love to hear some specific, less sweeping, comments about how  
the code is actually written and functions. Otherwise, I'll try to  
refactor this and return once again.


Thank you,
Chris Marcellino


On Feb 9, 2007, at 6:40 AM, Tom Lane wrote:


Chris Marcellino <[EMAIL PROTECTED]> writes:

Here is a new patch that uses the POSIX api's. It encodes the
canonical path (see 'man realpath') of the database's data directory
into the shared memory segment name using an strong hash function to
make it fit in the shared memory segment name under all cases,
without risk of key collision.


I find this patch utterly unreadable, because of your cavalier  
disregard
for making the comments match the truth.  You have copied-and- 
pasted the
original SysV code and fixed some small fraction of the comments,  
and I

cannot tell which ones still reflect reality --- but I can tell that a
lot of them don't.

Also, I don't see where this implements any sort of detection of live
backends attached to an existing segment, so I don't think you have
responded to that objection.  Magnus' idea for Windows was to use a
segment set up to automatically go away as soon as the last attacher
died, but AFAICT that isn't how this works.

regards, tom lane

---(end of  
broadcast)---

TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate



---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] Feature: POSIX Shared memory support, round 2

2007-02-09 Thread Tom Lane
Chris Marcellino <[EMAIL PROTECTED]> writes:
> Here is a new patch that uses the POSIX api's. It encodes the  
> canonical path (see 'man realpath') of the database's data directory  
> into the shared memory segment name using an strong hash function to  
> make it fit in the shared memory segment name under all cases,  
> without risk of key collision.

I find this patch utterly unreadable, because of your cavalier disregard
for making the comments match the truth.  You have copied-and-pasted the
original SysV code and fixed some small fraction of the comments, and I
cannot tell which ones still reflect reality --- but I can tell that a
lot of them don't.

Also, I don't see where this implements any sort of detection of live
backends attached to an existing segment, so I don't think you have
responded to that objection.  Magnus' idea for Windows was to use a
segment set up to automatically go away as soon as the last attacher
died, but AFAICT that isn't how this works.

regards, tom lane

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate