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

From Claudio Natoli
Subject Comments requested on attached fork/exec patch
Date
Msg-id A02DEC4D1073D611BAE8525405FCCE2B028078@harris.memetrics.local
Whole thread Raw
List pgsql-patches
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.com/em
ailpolicy.html</a>



Attachment

pgsql-patches by date:

Previous
From: Tom Lane
Date:
Subject: Re: Another pg_autovacuum patch
Next
From: "ViSolve Open Source Team"
Date:
Subject: Re: [BUGS] (Modified) Patch request for PostgreSQL 7.4 for HP-UX IA-64