Thread: 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.com/em
ailpolicy.html</a>
Attachment
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>
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>