805439 - TaskTree blocking indefinitely

Status: RESOLVED FIXED

(Marketplace Graveyard :: Search, defect)

Description

[Tarek Ziadé (:tarek)]

Using celery-tasktree for the reindexing feature works well, except it blocks at then end on the join because the client side sees all the tasks as 'PENDING' (while celeryd process them and mark them as succeeded)

Minimal bug example:


from celery_tasktree import task_with_callbacks, TaskTree

@task_with_callbacks
def one():
    return


def go():
    tree = TaskTree()
    tree.add_task(one)
    tree.apply_and_join() 


If you run it in manage shell, your can reproduce the issue

Comment 1

[Kumar McMillan [:kumar]]

We don't use apply_and_join() and I bet you it is broken because we throw away task results by default: https://github.com/mozilla/zamboni/blob/master/lib/settings_base.py#L1049 I believe you can override this on specific tasks but you might need to do it for each one.

btw, apply_and_join() is always a bad idea in a web request because it blocks. We use apply_async(). I see that you're doing it in a script so that makes sense; I bet no one has tried that yet.

Comment 2

[Tarek Ziadé (:tarek)]

> We don't use apply_and_join() and I bet you it is broken because we throw away task results by default

oh I will look at this thanks

>  btw, apply_and_join() is always a bad idea in a web request because it blocks.

It's only in the reindex management command where we want to block until it's finished

> I see that you're doing it in a script so that makes sense; I bet no one has tried that yet.

Yeah I assume it's better to have the script blocking, rather than logging the data into a log file and checking a flag if someone runs it twice etc

Comment 3

[Kumar McMillan [:kumar]]

I know it would be dumb but you could enter a while loop at the end and poll until it's finished. That might at least let you deploy the feature safely with task tree.

Yep, I'm pretty sure it needs tasks to return a result. I saw this in the task tree docs "If subtask (function) return value is an object, then a property named "async_result" will be added to that object so that it will be possible to join() for."

Comment 4

[Tarek Ziadé (:tarek)]

ok I've done this https://github.com/tarekziade/zamboni/commit/3eeaf0287871bfad0b0347b9211f8efef6ed2770

Comment 5

[Tarek Ziadé (:tarek)]

pushed at https://github.com/mozilla/zamboni/commit/7142b961a961460667ff5a3b97ba0509b04c2644