Re: [gentoo-portage-dev] PATCH: parallel-fetch
On Sat, Dec 17, 2005 at 12:02:00AM +0900, Jason Stubbs wrote:
> I mean for when a sub-process is running such as ebuild or rsync. Will
> portage
> get the SIGINT before or at the same time as the sub-process causing portage
> to then SIGKILL it while it's trying to exit cleanly? Given that portage's
> atexit handler is registered after portage_exec's, the subprocess reaping
> code would not have been doing anything in the past. I guess the question is
> more a general one rather than specifically related to background fetching.
Tests seem to work fine.
attached is updated parallel-fetch patch; main bits are chunking the
worthless atexit crap from portage.py .
So... test it out please. based against 2.1_pre1, although it should
apply to .53.
Thanks,
~harring
Index: bin/emerge
===
--- bin/emerge (revision 2390)
+++ bin/emerge (working copy)
@@ -1869,6 +1869,41 @@
self.pkgsettings["FEATURES"]=string.join(myfeat)
+ if "parallel-fetch" in myfeat and not ("--ask" in myopts or
"--pretend" in myopts or "--fetchonly" in myopts):
+ if "distlocks" not in myfeat:
+ print red("!!!")
+ print red("!!!")+" parallel-fetching requires
the distlocks feature enabled"
+ print red("!!!")+" you have it disabled, thus
parallel-fetching is being disabled"
+ print red("!!!")
+ elif len(mymergelist) > 1:
+ print ">>> starting parallel fetching"
+ pid = os.fork()
+ if not pid:
+ sys.stdin.close()
+ sys.stdout.close()
+ sys.stderr.close()
+ sys.stdout = open("/dev/null","w")
+ sys.stderr = open("/dev/null","w")
+ os.dup2(sys.stdout.fileno(), 1)
+ os.dup2(sys.stdout.fileno(), 2)
+ for x in ("autoaddcvs", "cvs"):
+ try:myfeat.remove(x)
+ except ValueError: pass
+ self.pkgsettings["FEATURES"] = "
".join(myfeat)
+ ret = 0
+ for x in mymergelist:
+ if x[0] != "ebuild":
+ continue
+ try:
+ ret =
portage.doebuild(portage.portdb.findname(x[2]), "fetch", x[1], self.pkgsettings,
+ cleanup=0,
fetchonly=True, tree="porttree")
+ except SystemExit:
+ raise
+ except Exception:
+ ret = 1
+ sys.exit(0)
+ portage.portage_exec.spawned_pids.append(pid)
+
mergecount=0
for x in mymergelist:
mergecount+=1
Index: pym/portage.py
===
--- pym/portage.py (revision 2390)
+++ pym/portage.py (working copy)
@@ -130,9 +130,6 @@
signal.signal(signal.SIGTERM, signal.SIG_IGN)
# 0=send to *everybody* in process group
- portageexit()
- print "Exiting due to signal"
- os.kill(0,signum)
sys.exit(1)
signal.signal(signal.SIGCHLD, signal.SIG_DFL)
@@ -6987,14 +6984,6 @@
def portageexit():
global uid,portage_gid,portdb,db
if secpass and not os.environ.has_key("SANDBOX_ACTIVE"):
- # wait child process death
- try:
- while True:
- os.wait()
- except OSError:
- #writemsg(">>> All child process are now dead.")
- pass
-
close_portdbapi_caches()
if mtimedb:
pgpf22JHBtdnM.pgp
Description: PGP signature
Re: [gentoo-portage-dev] PATCH: parallel-fetch
On Friday 16 December 2005 19:01, Brian Harring wrote: > On Fri, Dec 16, 2005 at 12:09:10PM +0900, Jason Stubbs wrote: > > On Thursday 15 December 2005 20:06, Brian Harring wrote: > > > This is the only blocker for merging parallel-fetch as far as I can > > > tell- so... my vote is nuking the wait out of portage.portageexit > > > (it's exec subsystem crap, belong in exec's atexit (where it exists)). > > > Assuming no complaints there, issues with parallel-fetch going into > > > svn? > > > > Nuking the wait is fine if things will still work the same.. I'm kind of > > wondering if ctrl-c'ing behaviour will change - how gets the ctrl-c > > first? > > ctrl-c doesn't come in play here- just triggers exithandler, which > calls portageexit. What's stupid about this code is that exithandler > has it's *own* set of kill calls in it (nothing like 101 duplicated > bits). > > Either way... exit, or nothing left to execute, python will then > trigger atexit registered callbacks in a lifo manner. So all > exithandler really _should_ have to do is just sys.exit. I mean for when a sub-process is running such as ebuild or rsync. Will portage get the SIGINT before or at the same time as the sub-process causing portage to then SIGKILL it while it's trying to exit cleanly? Given that portage's atexit handler is registered after portage_exec's, the subprocess reaping code would not have been doing anything in the past. I guess the question is more a general one rather than specifically related to background fetching. -- Jason Stubbs -- gentoo-portage-dev@gentoo.org mailing list
Re: [gentoo-portage-dev] PATCH: parallel-fetch
On Fri, Dec 16, 2005 at 12:09:10PM +0900, Jason Stubbs wrote: > On Thursday 15 December 2005 20:06, Brian Harring wrote: > > On Thu, Dec 01, 2005 at 07:41:22PM -0600, Brian Harring wrote: > > > > > > > Either way, here's the issue, atexit registers work fine across forks, > > > portage.portagexit is registered prior to portage_exec.cleanup, so the > > > main portage pid sits there and waits for the kids to go away on their > > > own, then portage_exec.cleanup tries waxing the pids. > > Ok.. After a few reads, I finally figured out what you're saying here. Yeah... email to ml was just a quick update from what I tracked down in irc, didn't realize the email was damn near unreadable till a while after it was sent (and no one had commented). > > This is the only blocker for merging parallel-fetch as far as I can > > tell- so... my vote is nuking the wait out of portage.portageexit > > (it's exec subsystem crap, belong in exec's atexit (where it exists)). > > Assuming no complaints there, issues with parallel-fetch going into > > svn? > > Nuking the wait is fine if things will still work the same.. I'm kind of > wondering if ctrl-c'ing behaviour will change - how gets the ctrl-c first? ctrl-c doesn't come in play here- just triggers exithandler, which calls portageexit. What's stupid about this code is that exithandler has it's *own* set of kill calls in it (nothing like 101 duplicated bits). Either way... exit, or nothing left to execute, python will then trigger atexit registered callbacks in a lifo manner. So all exithandler really _should_ have to do is just sys.exit. ~harring pgp5Om3dvsKCc.pgp Description: PGP signature
Re: [gentoo-portage-dev] PATCH: parallel-fetch
On Thursday 15 December 2005 20:06, Brian Harring wrote: > On Thu, Dec 01, 2005 at 07:41:22PM -0600, Brian Harring wrote: > > > > Either way, here's the issue, atexit registers work fine across forks, > > portage.portagexit is registered prior to portage_exec.cleanup, so the > > main portage pid sits there and waits for the kids to go away on their > > own, then portage_exec.cleanup tries waxing the pids. Ok.. After a few reads, I finally figured out what you're saying here. > This is the only blocker for merging parallel-fetch as far as I can > tell- so... my vote is nuking the wait out of portage.portageexit > (it's exec subsystem crap, belong in exec's atexit (where it exists)). > Assuming no complaints there, issues with parallel-fetch going into > svn? Nuking the wait is fine if things will still work the same.. I'm kind of wondering if ctrl-c'ing behaviour will change - how gets the ctrl-c first? -- Jason Stubbs -- gentoo-portage-dev@gentoo.org mailing list
Re: [gentoo-portage-dev] PATCH: parallel-fetch
On Thu, Dec 01, 2005 at 07:41:22PM -0600, Brian Harring wrote: > Either way, here's the issue, atexit registers work fine across forks, > portage.portagexit is registered prior to portage_exec.cleanup, so the > main portage pid sits there and waits for the kids to go away on their > own, then portage_exec.cleanup tries waxing the pids. This is the only blocker for merging parallel-fetch as far as I can tell- so... my vote is nuking the wait out of portage.portageexit (it's exec subsystem crap, belong in exec's atexit (where it exists)). Assuming no complaints there, issues with parallel-fetch going into svn? ~harring pgpBiVsRihrqW.pgp Description: PGP signature
Re: [gentoo-portage-dev] PATCH: parallel-fetch
On Thu, Dec 01, 2005 at 12:03:46PM -0800, Zac Medico wrote: > >>Well, I'm running the latest svn so there could be a regression in the > >>recent changes to portage_exec.cleanup(). > > > >Does changing SIKTERM to SIGKILL fix it? > > > > No, SIGKILL doesn't seem to make a difference (not a regression then). It > seems that the atexit hooks do not execute until the sys.exit() is called > in the fetcher child process (see attached test case that reproduces the > problem). Perhaps it would be more appropriate to implement parallel-fetch > with a thread rather than a fork? Thread'ing won't fly due to the code involved. Too many globals, too many structures screwed with- this is why I used fork (yes it's a mild hack, but it's so simple ;) Either way, here's the issue, atexit registers work fine across forks, portage.cleanup is registered prior to portage_exec.cleanup, so the main portage pid sits there and waits for the kids to go away on their own, then portage_exec.cleanup tries waxing the pids. Short version... cleanup's killing code never is needed at this point. So... who's up for yanking that os.wait out of portage.portageexit? :) ~harring pgpCLNFLAjoJG.pgp Description: PGP signature
Re: [gentoo-portage-dev] PATCH: parallel-fetch
Jason Stubbs wrote:
On Thursday 01 December 2005 10:27, Zac Medico wrote:
Brian Harring wrote:
2) Display a warning message via an atexit hook when parallel-fetching is
enabled, in order to alert the user that background fetching may _still_
be in progress if emerge appears to hang after an ebuild dies (this
happened to me while kde-3.5 was fetching in the background).
Details please...
atexit portage_exec hooks should kill off the fetcher.
Well, I'm running the latest svn so there could be a regression in the
recent changes to portage_exec.cleanup().
Does changing SIKTERM to SIGKILL fix it?
No, SIGKILL doesn't seem to make a difference (not a regression then). It
seems that the atexit hooks do not execute until the sys.exit() is called in
the fetcher child process (see attached test case that reproduces the problem).
Perhaps it would be more appropriate to implement parallel-fetch with a thread
rather than a fork?
Zac
#!/usr/bin/python
import os, sys
import portage, portage_exec
def child_process():
portage_exec.spawn("sleep 10")
sys.exit(0)
if __name__=="__main__":
pid = os.fork()
if not pid:
child_process()
else:
portage.portage_exec.spawned_pids.append(pid)
print "Calling sys.exit(1)"
sys.exit(1)
Re: [gentoo-portage-dev] PATCH: parallel-fetch
On Thursday 01 December 2005 10:27, Zac Medico wrote: > Brian Harring wrote: > >>2) Display a warning message via an atexit hook when parallel-fetching is > >>enabled, in order to alert the user that background fetching may _still_ > >> be in progress if emerge appears to hang after an ebuild dies (this > >> happened to me while kde-3.5 was fetching in the background). > > > > Details please... > > atexit portage_exec hooks should kill off the fetcher. > > Well, I'm running the latest svn so there could be a regression in the > recent changes to portage_exec.cleanup(). Does changing SIKTERM to SIGKILL fix it? -- Jason Stubbs -- gentoo-portage-dev@gentoo.org mailing list
Re: [gentoo-portage-dev] PATCH: parallel-fetch
Brian Harring wrote: 2) Display a warning message via an atexit hook when parallel-fetching is enabled, in order to alert the user that background fetching may _still_ be in progress if emerge appears to hang after an ebuild dies (this happened to me while kde-3.5 was fetching in the background). Details please... atexit portage_exec hooks should kill off the fetcher. Well, I'm running the latest svn so there could be a regression in the recent changes to portage_exec.cleanup(). Zac -- gentoo-portage-dev@gentoo.org mailing list
Re: [gentoo-portage-dev] PATCH: parallel-fetch
On Wed, Nov 30, 2005 at 12:51:58PM -0800, Zac Medico wrote: > Brian Harring wrote: > >Note that due to how it's implemented, this does two rounds of > >verification- it'll actually do *two* rounds of fetching too, if > >things go awry in the backgrounded thread. > > Two possible improvements to help prevent user confusion: > > 1) Display a warning message when waiting for distlocks and > parallel-fetching is enabled, in order to alert the user that background > fetching is likely in progress. portage_locks does so already. > 2) Display a warning message via an atexit hook when parallel-fetching is > enabled, in order to alert the user that background fetching may _still_ be > in progress if emerge appears to hang after an ebuild dies (this happened > to me while kde-3.5 was fetching in the background). Details please... atexit portage_exec hooks should kill off the fetcher. ~harring pgpNX9tteH9Ck.pgp Description: PGP signature
Re: [gentoo-portage-dev] PATCH: parallel-fetch
Brian Harring wrote: Note that due to how it's implemented, this does two rounds of verification- it'll actually do *two* rounds of fetching too, if things go awry in the backgrounded thread. Two possible improvements to help prevent user confusion: 1) Display a warning message when waiting for distlocks and parallel-fetching is enabled, in order to alert the user that background fetching is likely in progress. 2) Display a warning message via an atexit hook when parallel-fetching is enabled, in order to alert the user that background fetching may _still_ be in progress if emerge appears to hang after an ebuild dies (this happened to me while kde-3.5 was fetching in the background). Zac -- gentoo-portage-dev@gentoo.org mailing list
Re: [gentoo-portage-dev] PATCH: parallel-fetch
On Fri, Nov 18, 2005 at 11:09:14AM -0800, Zac Medico wrote: > Brian Harring wrote: > > >The modification is pretty straight forward offhand; the notable > >difference this time around is rather then extending portage_exec to > >have the capability to 'spawn' python funcs (something I always found > >ugly), this handles the fork itself. > > This patch seems to work well for me. I suggest that we add some sort of > logging ability so that possible fetch problems will be easier to diagnose. Note that due to how it's implemented, this does two rounds of verification- it'll actually do *two* rounds of fetching too, if things go awry in the backgrounded thread. Logging info is possible, but outputting to the term is nasty- need to basically have a message interface (imo) to dump it sanely, rather then dumping messages from the background fetcher whenever they arrive. Yes python locks print on it's own, but we also have a lot of sys.stdout abuse in use. Personally? Screw background logging. If it fails in the background fetcher, it'll rear it's head in the foreground giving immediate display. If you try to go through and integrate some type of output to the term for the background fetcher, you're going to have to do a *lot* of arg passing into and out of digest/fetcher to make it remain silent. I did this in ebd, and it was _not_ a simple patch. Cost outweighs the gain imo (hence the massively simplified patch for stable). Could keep the old stderr fd around, and write to it when a fetch has failed I spose, but again, don't think it's worth it. ~harring pgpI21IIrmOMr.pgp Description: PGP signature
Re: [gentoo-portage-dev] PATCH: parallel-fetch
Brian Harring wrote: The modification is pretty straight forward offhand; the notable difference this time around is rather then extending portage_exec to have the capability to 'spawn' python funcs (something I always found ugly), this handles the fork itself. This patch seems to work well for me. I suggest that we add some sort of logging ability so that possible fetch problems will be easier to diagnose. Zac -- gentoo-portage-dev@gentoo.org mailing list
[gentoo-portage-dev] PATCH: parallel-fetch
Yo.
Continuing the pillaging of ebd, attached is an integration of
parallel-fetch.
The modification is pretty straight forward offhand; the notable
difference this time around is rather then extending portage_exec to
have the capability to 'spawn' python funcs (something I always found
ugly), this handles the fork itself.
Debatable on that one, so feedback welcome.
~harring
Index: bin/emerge
===
--- bin/emerge (revision 2309)
+++ bin/emerge (working copy)
@@ -1821,6 +1824,42 @@
self.pkgsettings["FEATURES"]=string.join(myfeat)
+ if "parallel-fetch" in myfeat and not ("--ask" in myopts or
"--pretend" in myopts or "--fetchonly" in myopts):
+ if "distlocks" not in myfeat:
+ print red("!!!")
+ print red("!!!")+" parallel-fetching requires
the distlocks feature enabled"
+ print red("!!!")+" you have it disabled, thus
parallel-fetching is being disabled"
+ print red("!!!")
+ elif len(mymergelist) > 1:
+ print ">>> parallel-fetching is in the hizzay"
+ pid = os.fork()
+ if not pid:
+ sys.stdin.close()
+ sys.stdout.close()
+ sys.stderr.close()
+ sys.stdout = open("/dev/null","w")
+ sys.stderr = open("/dev/null","w")
+ os.dup2(sys.stdout.fileno(), 1)
+ os.dup2(sys.stdout.fileno(), 2)
+ for x in ("autoaddcvs", "cvs"):
+ try:myfeat.remove(x)
+ except ValueError: pass
+ self.pkgsettings["FEATURES"] = "
".join(myfeat)
+ print "child"
+ ret = 0
+ for x in mymergelist:
+ if x[0] != "ebuild":
+ continue
+ try:
+ ret =
portage.doebuild(portage.portdb.findname(x[2]), "fetch", x[1], self.pkgsettings,
+ cleanup=0,
fetchonly=True, tree="porttree")
+ except SystemExit:
+ raise
+ except Exception:
+ ret = 1
+ sys.exit(0)
+ portage.portage_exec.spawned_pids.append(pid)
+
mergecount=0
for x in mymergelist:
mergecount+=1
pgp7U7HSghoAe.pgp
Description: PGP signature
