Django connection issue with RTM v2 client #1238

Activity

1. seratch

   changed the title [-]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?[/-] [+]Django connection issue with RTM v2 client[/+] on Jul 15, 2022
2. seratch

   added

   Version: 2x

   enhancementM-T: A feature request for new functionalityM-T: A feature request for new functionality

   rtm-client

   on Jul 15, 2022
3. seratch

   self-assigned this

   on Jul 15, 2022
4. seratch

   added this to the 3.18.0 milestone on Jul 15, 2022
5. seratch commented on Jul 15, 2022

   

   seratch

   on Jul 15, 2022

   ContributorAuthor

   More actions

   Hi @pbrackin, thanks for sharing this. The connection releasing logic may not work in the case where your message listener receives a message from its bot user itself: https://github.com/slackapi/python-slack-sdk/blob/v3.17.2/slack_sdk/rtm_v2/__init__.py#L162-L164

   We are happy to add some customization points for this use case but, can you verify whether the approach works for you?

   • Copy https://github.com/slackapi/bolt-python/blob/v1.14.1/slack_bolt/adapter/django/handler.py#L64-L71 into your code
   • Copy https://github.com/slackapi/python-slack-sdk/blob/v3.17.2/slack_sdk/rtm_v2/__init__.py#L291-L307 and edit process_message method as below

       def process_message(self):
           release_thread_local_connections()  # ADDED
           try:
               raw_message = self.message_queue.get(timeout=1)
               if self.logger.level <= logging.DEBUG:
                   self.logger.debug(f"A message dequeued (current queue size: {self.message_queue.qsize()})")
   
               if raw_message is not None:
                   message: dict = {}
                   if raw_message.startswith("{"):
                       message = json.loads(raw_message)
   
                   def _run_message_listeners():
                       self.run_message_listeners(message)
   
                   self.message_workers.submit(_run_message_listeners)
           except Empty:
               pass
           finally:  # ADDED
               release_thread_local_connections()  # ADDED

   If this works well, we will add two callbacks like on_process_message_start and on_process_message_end to run release_thread_local_connections() without modifying the RTM client code.
6. seratch

   added

   Version: 3x

   and removed

   Version: 2x

   on Jul 16, 2022
7. seratch

   modified the milestones: 3.18.0, 3.x on Jul 19, 2022
8. pbrackin commented on Jul 30, 2022

   

   pbrackin

   on Jul 30, 2022

   More actions

   Thanks @seratch for following up on this. Sorry I didn't get to respond a little sooner. I will run this experiment when I get some time. But I wanted to also point out that I found a solution that seems to work ok. What I did was call connections.close_all() at the end of my message handler. Also I call close_old_connections() in my main loop which runs every 15 secs atm. We use MySQL for this project, and so I figured out that running sudo mysqladmin -i 1 processlist was a good way to see what is happening with the DB connections. Anyway, with those 2 Django calls in place, the connection list stopped growing with new messages coming in, and I no longer get the OperationalError(s) like "MySQL has gone away".
   Before I figured out this solution, I was about to just move all the incoming messages to a Q and have the processing all done by a dedicated processing thread - which would probably be ideal from a DB connection standpoint since that would allow Django to recycle connections in the pool. The cost of that would be some additional delay in the processing for Q / d-Q stuff. But with my solution, everything seems ok now.
   Anyway - let me know what you think of my approach. I will get back on adding in the code changes you suggested.
9. seratch commented on Aug 1, 2022

   

   seratch

   on Aug 1, 2022

   ContributorAuthor

   More actions

   @pbrackin Thanks for your reply. Most of the things you mentioned should be good ways to go but

   What I did was call connections.close_all() at the end of my message handler.

   This should be close_old_connections for safety. Refer to slackapi/bolt-python#509 for more details.

   Let me know if my above suggestions work for you. Once it is effective, we are happy to enhance the RTM v2 client to enable developers to pass callback functions for this use case.