PR#3072: Make sure session is cleaned up even if task raises exception during execution -

pagure

#3072 Make sure session is cleaned up even if task raises exception during execution

Merged Mar 16, 2018 by pingou. Opened Mar 16, 2018 by bkabrda.

bkabrda/pagure tasks-cleanup-session into master

bkabrda commented Mar 16, 2018

I have a Pagure deployment that had disk outage during night. The worker was trying to create new projects, but due to the disk outage, the task always failed. The problem was that it failed only after it created the session and due to the way tasks are implemented now, it didn't close the session. When the storage came back up, the worker couldn't recover, because the session pool was exhausted and no new sessions could be created.

This PR generally fixes situations when a task raises in the middle and doesn't do session cleanup.

I'm running tests with this change applied right now. I'll update this issue once I'm sure this didn't break anything.

pingou commented Mar 16, 2018

Good fix for me, thanks!

I'll wait for your output on the test results before merging :)

bkabrda commented Mar 16, 2018

So I am getting some failures, but they seem to be issues related to [1], so again the too-many-open-files problem. When I rerun these tests individually, they pass. So IMO this should be safe to merge.

[1] https://github.com/celery/celery/issues/2373

pingou commented Mar 16, 2018

Thanks for the feedback :)

Pull-Request has been merged by pingou

Mar 16, 2018

Metadata

Assignee

None

Tags

No Tags

pagure

#3072 Make sure session is cleaned up even if task raises exception during execution Merged Mar 16, 2018 by pingou. Opened Mar 16, 2018 by bkabrda. bkabrda/pagure tasks-cleanup-session into master

Metadata

#3072 Make sure session is cleaned up even if task raises exception during execution

Merged Mar 16, 2018 by pingou. Opened Mar 16, 2018 by bkabrda.

bkabrda/pagure tasks-cleanup-session into master