Re: Comments requested on attached fork/exec patch - Mailing list pgsql-patches

Sorry all... I'll repost with a context diff when I get a chance.

-----Original Message-----
From: Claudio Natoli
To: 'pgsql-patches@postgresql.org'
Sent: 12/10/03 12:28 AM
Subject: [PATCHES] Comments requested on attached fork/exec patch


Hi all,

here's a patch aimed at moving along the fork/exec development. It is
not
yet complete or ready for submission, but would like comments at this
stage
from the postgres developer community.

The places of particular interest are denoted by "FIXME: [fork/exec]",
however, some things I'd like to highlight are:

    * BackendFork changes are aimed at making BackendFork, in the
fork/exec case, simply format arguments in a manner which would be safe
for
the postmaster itself to call, deferring calls to ProcessStartupPacket
and
the like to PostgresMain (which is run by the exec'd child)
    - ultimate goal is to move the fork call into BackendFork
itself,
and, in the fork/exec case, allowing the postmaster to run the body of
the
function, and then fork/exec'ing at the very end of the routine
    - this is necessary for the Win32 port, because the Win32
CreateProcess call will effectively replace both the fork + exec calls
(ie.
we can't really have any processing between the fork + exec calls, as
any
context available under *nix after the fork call just has no analogy
under
Win32).

    * Things like FreeSpaceMap, shmInvalBuffer and PMSignalFlags are
now
registered in ShmemIndex via ShmemInitStruct. Suggest having their
respective Init* functions take a boolean "init" parameter for sanity
checking (as per StrategyInitialize)

    * Lock/ProcLock pulled out of LockMethodTable (?) and
FreeSpaceMap->relHash moved to stand-alone var. This is because the
memory
corresponding to the hash tables headers which they point to is not
accessible by backends in the fork/exec case

    * Getting rid of the (seemingly paranoid) locking in buf_init.c
and
lock.c would be a help ("Can we remove this" comments in patch)

    * Changed ShmemIndexLock to be a spin lock in its own right. But
could/should we just use ShmemLock instead?

    * Statement     /* ShmemIndex can't be set up yet (need LWLocks
first) */
       inside InitShmemAllocation is no longer true. Should we now
init
ShmemIndex here? Seems like the reasonable thing to do...

    * Polluting -p argument list. Bruce + I have discussed
writing/reading these from file as per the guc variables. Refer to
read/write_backend_variables functions.

    * AttachSharedMemoryAndSemaphores looking more and more like
CreateSharedMemoryAndSemaphores. Refactor to one function?

    * A number of comments (related to say, where ShmemIndex
can/can't
be initialized, or about fork/exec) need changing...

FWIW, this patch appears to have no detrimental effect on the existing
(non
EXEC_BACKEND) code base (based solely on passing 'make check' under
cygwin).
It does appear to give allow more or less workable fork/exec'ing of
backends
(EXEC_BACKEND defined) but it still needs some work.

Whilst this patch targets BackendFork, if I can get it into shape for
submission I'm keen to move onto continuing/finishing off the remaining
fork/exec work (namely, the stats buffer + collector processes, and the
forking from SSDataBase).

Cheers,
Claudio


---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.co
m/em
ailpolicy.html</a>


 <<diff.out>>  <<ATT15757.txt>>

---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em
ailpolicy.html</a>