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

Context diff attached.


> -----Original Message-----
> From: Claudio Natoli [mailto:claudio.natoli@memetrics.com]
> Sent: Wednesday, 10 December 2003 12:28 AM
> To: 'pgsql-patches@postgresql.org'
> 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.me
> metrics.com/em
> ailpolicy.html</a>
>
>
>

---
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>