Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add worker process lifetime to multiprocessing.Pool - patch included #51212

Closed
charlesc mannequin opened this issue Sep 22, 2009 · 18 comments
Closed

Add worker process lifetime to multiprocessing.Pool - patch included #51212

charlesc mannequin opened this issue Sep 22, 2009 · 18 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@charlesc
Copy link
Mannequin

charlesc mannequin commented Sep 22, 2009

BPO 6963
Files
  • worker-lifetime-python2.6.2.patch: 4th version of patch adding worker lifetime to multiprocessing.Pool
  • mp_patch_6963_trunk.patch: trunk based patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2010-01-27.03:36:56.117>
    created_at = <Date 2009-09-22.02:28:11.774>
    labels = ['type-feature', 'library']
    title = 'Add worker process lifetime to multiprocessing.Pool - patch included'
    updated_at = <Date 2010-01-27.03:36:56.116>
    user = 'https://bugs.python.org/charlesc'

    bugs.python.org fields:

    activity = <Date 2010-01-27.03:36:56.116>
    actor = 'jnoller'
    assignee = 'jnoller'
    closed = True
    closed_date = <Date 2010-01-27.03:36:56.117>
    closer = 'jnoller'
    components = ['Library (Lib)']
    creation = <Date 2009-09-22.02:28:11.774>
    creator = 'charlesc'
    dependencies = []
    files = ['15940', '16004']
    hgrepos = []
    issue_num = 6963
    keywords = ['patch', 'needs review']
    message_count = 18.0
    messages = ['92971', '92987', '92999', '93191', '93517', '93916', '93920', '93921', '93923', '95243', '95250', '97832', '97835', '98032', '98129', '98313', '98382', '98402']
    nosy_count = 2.0
    nosy_names = ['jnoller', 'charlesc']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue6963'
    versions = ['Python 2.7', 'Python 3.2']

    @charlesc
    Copy link
    Mannequin Author

    charlesc mannequin commented Sep 22, 2009

    Worker processes with multiprocessing.Pool live for the duration of the
    Pool. If the tasks they run happen to leak memory (from a C extension
    module, or from creating cycles of unreachable objects, etc) or open
    files or other resources, there's no easy way to clean them up.

    Similarly, if one task passed to the pool allocates a large amount of
    memory, but further tasks are small, that additional memory isn't
    returned to the system because the process involved hasn't exited.

    A common approach to this problem (as used by Apache, mod_wsgi, and
    various other software) is to allow worker processes to exit (and be
    replaced with fresh processes) after completing a specified amount of
    work. The attached patch (against Python 2.6.2, but applies to various
    other versions with some fuzz) implements this as optional new behaviour
    in multiprocessing.Pool(). An additional optional argument is specified
    for the maximum number of tasks a worker process performs before it
    exits and is replaced with a fresh worker process.

    @charlesc charlesc mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Sep 22, 2009
    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Sep 22, 2009

    Hi Charles; I don't see a doc update for this (see multiprocessing.rst) or
    unit tests. I'm not against it, but before I can commit this, I'll need
    those things

    @charlesc
    Copy link
    Mannequin Author

    charlesc mannequin commented Sep 22, 2009

    Alright, I'll add those. Thanks for looking at it; first pass was
    mostly to ensure it wouldn't be wasted work.

    @charlesc
    Copy link
    Mannequin Author

    charlesc mannequin commented Sep 28, 2009

    Updated patch attached; handles some of the Pool methods that weren't
    handled before. Now includes documentation and unit test additions as well.

    @charlesc
    Copy link
    Mannequin Author

    charlesc mannequin commented Oct 4, 2009

    Jesse: this is ready for your review now. Thanks,

    Charles

    @charlesc
    Copy link
    Mannequin Author

    charlesc mannequin commented Oct 13, 2009

    Can someone review this patch? I believe it's sufficient for inclusion
    now, as it includes docs and unit tests, but if anything about it
    requires further attention I'd be happy to listen to change requests.
    We'd like get this into mainline where everyone can benefit rather than
    continuing to maintain it out-of-tree.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Oct 13, 2009

    I plan on reviewing the patch once my work with PyCon is completed, which
    should be within the next few weeks. The next release this would show up
    in (2.7/3.2) is not for some time.

    @charlesc
    Copy link
    Mannequin Author

    charlesc mannequin commented Oct 13, 2009

    Okay, thanks, Jesse. Didn't realize the Con was on.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Oct 13, 2009

    Well, it's in feb, but I'm involved in some intense planning right now.

    @charlesc
    Copy link
    Mannequin Author

    charlesc mannequin commented Nov 14, 2009

    Hi Jesse -- Any chance you'll be able to review this in time for it to
    make it into trunk for the 2.7 alpha release?

    Charles

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Nov 14, 2009

    On Sat, Nov 14, 2009 at 11:43 AM, Charles Cazabon
    <report@bugs.python.org> wrote:

    Charles Cazabon <charlesc-python@pyropus.ca> added the comment:

    Hi Jesse -- Any chance you'll be able to review this in time for it to
    make it into trunk for the 2.7 alpha release?

    2.7 isn't slated until next year some time, so yes.

    @charlesc
    Copy link
    Mannequin Author

    charlesc mannequin commented Jan 15, 2010

    Ping... two alphas into 2.7. Have you had a chance to review this functionality?

    Thanks.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jan 15, 2010

    I'm fine with the functionality; I'm going to test it out and look at committing it by mid-week next week. I apologize, I've been pretty maxed out.

    @charlesc
    Copy link
    Mannequin Author

    charlesc mannequin commented Jan 18, 2010

    No problem, Jesse, I realize you're busy.

    I've updated the patch very slightly to handle an issue I had where the worker maintainer hung once.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jan 22, 2010

    I'm working on this now; I'm going to need to port the patch to trunk before moving forward with it. Shouldn't take me long.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jan 26, 2010

    Attached is the ported patch for Python 2.7/trunk. Please review it to make sure I didn't completely flub anything. I noticed you had forgotten the maxtasksperchild argument in the unit test, so I added that. I also expanded the docs a little but, paraphrasing your original post w.r.t to the justification.

    Let me know what you think, or if you have issues. Once you give the go ahead, I will commit to trunk and then merge to Py3k.

    Once again, thanks for the patch

    @charlesc
    Copy link
    Mannequin Author

    charlesc mannequin commented Jan 26, 2010

    Thanks, Jesse -- it looks good. If there are bugs remaining in the patch, they're mine and not yours.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jan 27, 2010

    Committed to trunk in r77794
    Merged to Py3k in r77795

    @jnoller jnoller mannequin closed this as completed Jan 27, 2010
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    0 participants