Skip to content

Fixed event capture from building infinite list#5870

Merged
auvipy merged 1 commit into
celery:masterfrom
Prodge:patch-1
Feb 9, 2020
Merged

Fixed event capture from building infinite list#5870
auvipy merged 1 commit into
celery:masterfrom
Prodge:patch-1

Conversation

@Prodge

@Prodge Prodge commented Dec 12, 2019

Copy link
Copy Markdown
Contributor

consume is a generator returning an infinite list of Nones.

Wrapping this in a list will slowly build this list, holding it in memory.

This is a very slow leak and is only visible over some time when there is a large volume of events.

Supersedes #5482.

Fixes #4843.

@auvipy auvipy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a test?

@thedrow

thedrow commented Dec 15, 2019

Copy link
Copy Markdown
Contributor

@auvipy What kind of test do you have in mind?

@auvipy

auvipy commented Dec 16, 2019

Copy link
Copy Markdown
Member

this can be addressed in point release.

@auvipy auvipy modified the milestones: 4.4.0, 4.4.x Dec 16, 2019
@drsantos20

Copy link
Copy Markdown

@auvipy What kind of test do you have in mind?

Do you know if we already have a test for it? if yes you can implement covering what you added

Comment thread celery/events/receiver.py
forced via :exc:`KeyboardInterrupt` or :exc:`SystemExit`.
"""
return list(self.consume(limit=limit, timeout=timeout, wakeup=wakeup))
for _ in self.consume(limit=limit, timeout=timeout, wakeup=wakeup):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although, the fix seems correct, I could not find a connection with worker functionality. Event capturing is performed by specific commands only. Could you please explain how this method affects the worker? Thanks!

@spidernello

spidernello commented Jan 28, 2020

Copy link
Copy Markdown

@Prodge a feedback on this one would be highly appreciated! we still experience memory leak issues and hoping a permanent fix is being released, thanks

@auvipy

auvipy commented Jan 29, 2020

Copy link
Copy Markdown
Member

@Prodge a feedback on this one would be highly appreciated! we still experience memory leak issues and hoping a permanent fix is being released, thanks

can you try this patch?

@spidernello

spidernello commented Jan 30, 2020

Copy link
Copy Markdown

I'm evaluating the possibility to patch staging as well as upgrading to celery 4.4.0 (have reviewed the changelog and seen some memory leak fixes have been applied to that version https://github.com/celery/celery/releases/tag/4.4.0 - want to identify if those are fixing our problem or not) and perform some tests, I'll update you as soon as I get to that

@spidernello

Copy link
Copy Markdown

I'm about to test the patch in our version of celery 4.2.1 in staging. I'll give some updates as soon as I have some feedback

@spidernello

Copy link
Copy Markdown

I can confirm the proposed patch fixed the memory leak, we run some tests over the weekend and did not see any memory leak occurring

@auvipy
auvipy merged commit f2ddd89 into celery:master Feb 9, 2020
@minbong728

Copy link
Copy Markdown

@auvipy Do you know when we can expect to see this in an official release?

@spidernello

Copy link
Copy Markdown

I also wanted to share with you that once I applied this flag --without-gossip --without-mingle --without-heartbeat in the celery multi string I have seen a general improvement in the performances celery <--> rabbitmq - on the rabbitmq side we often were seeing disconnection and very high cpu utilisation. After applying this fix all these behaviours disappeared Hope this helps

@auvipy

auvipy commented Feb 26, 2020

Copy link
Copy Markdown
Member

@auvipy Do you know when we can expect to see this in an official release?

ASAP! It may be the last day of Feb!