Skip to content

Fixed a possible memory leak in worker #5482

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.

Sign up for GitHub

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

Jump to bottom
Closed
auvipy wants to merge 1 commit into from
Closed

Fixed a possible memory leak in worker #5482

auvipy wants to merge 1 commit into from

Conversation

@auvipy
Copy link
Member

@auvipy auvipy commented Apr 24, 2019

No description provided.

@auvipy auvipy added this to the 4.3.x Maintenance milestone Apr 24, 2019
@auvipy auvipy mentioned this pull request Apr 24, 2019
Open
@yevhen-m
Copy link

yevhen-m commented Apr 24, 2019

According to #4843 (comment) only eventlet/gevent workers have this issue. I don't understand this comment, but it looks like your change does not fix the root cause:

            ...
            # we currently only buffer events when the event loop is enabled
            # XXX This excludes eventlet/gevent, which should actually buffer.

@auvipy
Copy link
Member Author

auvipy commented Apr 24, 2019

this is just a copy of your proposed fix in your fork

@yevhen-m
Copy link

yevhen-m commented Apr 24, 2019

@auvipy I see, and it's good for me ;). I'm not 100% sure that it's good for everyone else.

@codecov
Copy link

codecov bot commented Apr 24, 2019 •
edited
Loading

Codecov Report

Merging #5482 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5482   +/-   ##
=======================================
  Coverage   82.79%   82.79%           
=======================================
  Files         144      144           
  Lines       16492    16492           
  Branches     2057     2057           
=======================================
  Hits        13655    13655           
  Misses       2625     2625           
  Partials      212      212
Impacted Files Coverage Δ
celery/worker/consumer/events.py 96.77% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d38770...25760ed. Read the comment docs.

@auvipy
Copy link
Member Author

auvipy commented Apr 24, 2019

I opened the pr for more consensus actually

@auvipy auvipy requested review from othalla and thedrow May 2, 2019 19:29
@auvipy
Copy link
Member Author

auvipy commented May 5, 2019

OK let's postpone this for a while!

@auvipy auvipy self-assigned this May 6, 2019
@auvipy auvipy requested a review from a team June 14, 2019 22:01
@auvipy auvipy modified the milestones: 4.4.0, 4.5 Jun 14, 2019
@jezdez
Copy link
Contributor

jezdez commented Jun 14, 2019

@auvipy @thedrow Forgive me if I'm wrong, but I'm confused why the technical board team was requested to review this pull request (and others).

celery/ceps#13 and CEP 1 described the technical board as solely for high-level (read: CEP-level) decision making to support the core developers, not day-to-day development. This may have been a mistake only, but please let me know it the CEP isn't up-to-date anymore.

@auvipy
Copy link
Member Author

auvipy commented Jun 14, 2019

the CEP is alright. I am the culprit to request the review here!!

@matteius
Copy link
Contributor

matteius commented Jun 29, 2019

TEST?

@matteius
Copy link
Contributor

matteius commented Jun 29, 2019

What is the outcome when the connection is offline and it will now raise from within publish? What is expected happens next in the exception handling by default?

@Prodge Prodge mentioned this pull request Dec 15, 2019
Merged
@spidernello
Copy link

spidernello commented Jan 28, 2020

Team, is there any update about this issue? Was this addressed and fixed in celery 4.4.0?

@auvipy
Copy link
Member Author

auvipy commented Jan 28, 2020

can you check this #5870 ?

@spidernello
Copy link

spidernello commented Jan 28, 2020

I see , so it seems it is still waiting for a test to be executed and it is still not merged in, I can see the PR is open , correct?

@auvipy
Copy link
Member Author

auvipy commented Jan 28, 2020

you should try that patch on your staging env

@spidernello
Copy link

spidernello commented Jan 29, 2020

@auvipy thanks for your answer. To be precise this is the type of exception we found, I looked at the solution proposed in the PR and would like to understand if its applicable also to our problem based on the error log or its unrelated issue

Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/celery/worker/worker.py", line 205, in start
    self.blueprint.start(self)
  File "/usr/lib/python3.6/site-packages/celery/bootsteps.py", line 119, in start
    step.start(parent)
  File "/usr/lib/python3.6/site-packages/celery/bootsteps.py", line 369, in start
    return self.obj.start()
  File "/usr/lib/python3.6/site-packages/celery/worker/consumer/consumer.py", line 317, in start
    blueprint.start(self)
  File "/usr/lib/python3.6/site-packages/celery/bootsteps.py", line 119, in start
    step.start(parent)
  File "/usr/lib/python3.6/site-packages/celery/worker/consumer/consumer.py", line 593, in start
    c.loop(*c.loop_args())
  File "/usr/lib/python3.6/site-packages/celery/worker/loops.py", line 91, in asynloop
    next(loop)
  File "/usr/lib/python3.6/site-packages/kombu/asynchronous/hub.py", line 293, in create_loop
    poll_timeout = fire_timers(propagate=propagate) if scheduled else 1
  File "/usr/lib/python3.6/site-packages/kombu/asynchronous/hub.py", line 136, in fire_timers
    entry()
  File "/usr/lib/python3.6/site-packages/kombu/asynchronous/timer.py", line 68, in __call__
    return self.fun(*self.args, **self.kwargs)
  File "/usr/lib/python3.6/site-packages/kombu/asynchronous/timer.py", line 127, in _reschedules
    return fun(*args, **kwargs)
  File "/usr/lib/python3.6/site-packages/billiard/pool.py", line 1316, in maintain_pool
    sys.exc_info()[2])
  File "/usr/lib/python3.6/site-packages/billiard/five.py", line 126, in reraise
    raise value.with_traceback(tb)
  File "/usr/lib/python3.6/site-packages/billiard/pool.py", line 1307, in maintain_pool
    self._maintain_pool()
  File "/usr/lib/python3.6/site-packages/billiard/pool.py", line 1299, in _maintain_pool
    self._repopulate_pool(joined)
  File "/usr/lib/python3.6/site-packages/billiard/pool.py", line 1284, in _repopulate_pool
    self._create_worker_process(self._avail_index())
  File "/usr/lib/python3.6/site-packages/celery/concurrency/asynpool.py", line 449, in _create_worker_process
    return super(AsynPool, self)._create_worker_process(i)
  File "/usr/lib/python3.6/site-packages/billiard/pool.py", line 1116, in _create_worker_process
    w.start()
  File "/usr/lib/python3.6/site-packages/billiard/process.py", line 124, in start
    self._popen = self._Popen(self)
  File "/usr/lib/python3.6/site-packages/billiard/context.py", line 333, in _Popen
    return Popen(process_obj)
  File "/usr/lib/python3.6/site-packages/billiard/popen_fork.py", line 24, in __init__
    self._launch(process_obj)
  File "/usr/lib/python3.6/site-packages/billiard/popen_fork.py", line 72, in _launch
    self.pid = os.fork()
MemoryError: [Errno 12] Cannot allocate memory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@othalla othalla Awaiting requested review from othalla

Assignees

@auvipy auvipy

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@auvipy @yevhen-m @jezdez @matteius @spidernello