[Pulp-list] pulp threading

[Jason L Connor]

On Mon, 2011-06-06 at 10:46 -0500, Jeff Ortel wrote:
> All,
> 
> In tasking/thread.py:99 we are replacing Thread class with our own:
> 
> <snip>
> # monkey-patch the threading module in order to track threads
> # this allows us to cancel tasks that have spawned threads of their
> own
> threading.Thread = TrackedThread
> </snip>
> 
> Although, I don't see anything in the TrackedThread class or in this
> module that would 
> directly cause problems, I have concerns about tampering with the core
> python threading 
> module.  It appears that the purpose for doing this is to add support
> for cancelling 
> (killing) threads primarily in support of our task subsystem.  Since
> TaskThread already 
> extends TrackedThread, I don't see any advantage to modifying the
> threading module 
> (although I could be missing something).  Replacing the Thread class
> in the threading 
> module potentially affects all threads such as those spawned by QPID,
> gofer, grinder and 
> perhaps mod_wsgi itself which may be having unintended side effects.
> 
> Since we are seeing weirdness wrt threading/locking while running
> within mod_wsdi, I 
> suggest that unless reassigning the Thread class within the threading
> module is absolutely 
> essential to tasking, that we remove the above code snip.  Again, the
> code involved here 
> doesn't seem to do anything bad but if nothing else, I'd like to
> remove it as a possible 
> cause.
> 
> Comments? 

If we're not using it, I don't mind removing it.

However, this monkey patch has been in place forever. I think we need
actual evidence as to what the problems are and this feels like guess
work.

-- 
Jason L Connor
linear on freenode #pulp
http://pulpproject.org/
RHCE: 805010912355231
GPG Fingerprint: 2048R/CC4ED7C1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part
URL: <http://listman.redhat.com/archives/pulp-list/attachments/20110606/58da5532/attachment.sig>