search

slackapi / bolt-python

A framework to build Slack apps using Python
https://tools.slack.dev/bolt-python/
MIT License
1.06k stars 245 forks source link

Django thread-local connection cleanup in multi threads #280

Closed seratch closed 3 years ago

seratch commented 3 years ago

This issue illustrates a potential issues that may affect Django adapter users.

Django's ORM establishes a database connection per thread. For the threads managed by Django (which are supposed to be used for directly handing web requests), Django ORM cleans the established connections up under the hood once the processing on the thread completes (= request_finished signal).

On the other hand, in the case where Django ORM models are used in an unmanaged thread, Django does not handle the cleanup of the connections bound to the thread although the Django framework automatically associates a new connection to the thread. This half-heartedly maintained resource possibly may cause stale DB connection related issues.

To learn more details of this Django behavior, checking the following ticket is helpful for understanding this: https://code.djangoproject.com/ticket/9878

As Bolt for Python utilizes threads for enabling developers to run ack() method asynchronously (as long as process_before_response=False, which is the default), this framework should have a special treatment for this possible issue with Django.

As the solution for this issue (and possible similar needs), we'll introduce a new completion callback to the ListenerRunner mechanism. The handler will be quite similar to the ListenerErrorHandler callback but the given callback function is executed in the finally clause of listener runner invocation: https://github.com/slackapi/bolt-python/blob/v1.4.4/slack_bolt/listener/thread_runner.py#L99-L126 Also, for this Django specific issue, we'll add a custom lazy listener runner to the Django adapter module.

I've already implemented the initial version of this. We can discuss its details in my upcoming pull request.

pbrackin commented 2 years ago

We are refitting to use the v2 RTM client and still observe some stale Django connection issues. Ie, new DB connections are made for each message that gets processed (thread), but they are apparently not being released. I have tried using the close_old_connections & also ensure_connection, but seems like it doesn't really help. I saw you did the changes for the slack-bolt thing here. Were there going to be similar changes to the v2 RTM client? Do you have any recommendations in the interim?

seratch commented 2 years ago

@pbrackin Let's discuss this in python-slack-sdk project. I will start a new issue for it.