[mythtv-commits] Ticket #13320: Python bindings not reconnecting to database after a long pause.

[MythTV]

#13320: Python bindings not reconnecting to database after a long pause.
----------------------------------+--------------------------------
 Reporter:  sir-maniac            |          Owner:  Raymond Wagner
     Type:  Bug Report - General  |         Status:  new
 Priority:  minor                 |      Milestone:  needs_triage
Component:  Bindings - Python     |        Version:  Unspecified
 Severity:  medium                |     Resolution:
 Keywords:                        |  Ticket locked:  0
----------------------------------+--------------------------------

Comment (by rcrdnalor):

 It seems to me, that this bug hides additional bugs:

 For convenience, I add this piece of code in question here:
 https://github.com/MythTV/mythtv/blob/master/mythtv/bindings/python/MythTV/_conn_mysqldb.py#L29
 {{{
 class LoggedCursor( MySQLdb.cursors.Cursor ):
     """
     Custom cursor, offering logging and error handling
     """
     def __init__(self, connection):
         super(LoggedCursor, self).__init__(connection)
         self.log = None
         self.ping = ref(self._ping121)
         if MySQLdb.version_info >= ('1','2','2'):
             self.ping = ref(self._ping122)
         self.ping()

     def _ping121(self): self._get_db().ping(True)
     def _ping122(self): self._get_db().ping()
 }}}

 1) According help of the python's MySQLdb driver, the statement
 `ping(True)` was implemented in version 1.2.2.
 See
 {{{
 >>> import MySQLdb
 >>> help(MySQLdb._mysql.connection.ping)
 Help on method_descriptor:

 ping(...)
     Checks whether or not the connection to the server is
     working. If it has gone down, an automatic reconnection is
     attempted.

     This function can be used by clients that remain idle for a
     long while, to check whether or not the server has closed the
     connection and reconnect if necessary.

     New in 1.2.2: Accepts an optional reconnect parameter. If True,
     then the client will attempt reconnection. Note that this setting
     is persistent. By default, this is on in MySQL<5.0.3, and off
     thereafter.

     Non-standard. You should assume that ping() performs an
     implicit rollback; use only when starting a new transaction.
     You have been warned
 }}}
 Note: The code in question implies the opposite.

 2) I wrote a small test (see attachment), that checks the behavior of a
 timed-out connection from MythTV's python bindings to mysql.
 Note, that the default timeouts seem to be 8 hours, therefore I needed to
 shorten them.
 But other installations may override this to a much smaller value.

 If I use `_ping122(), the timed out connection raises an uncaught
 exception:
 {{{
 _mysql_exceptions.OperationalError: (2006, 'MySQL server has gone away')
 }}}

 If I use `_ping121(), the timed out connection reconnects to mysql.
 This makes me think, that the ping with the `True` argument should be used
 (`_ping121()`), even on systems with never versions of MySQLdb.

 Note, that the statement about the reconnect setting in the help of
 `ping()` is not entirely true:
 {{{
 By default, this is on in MySQL<5.0.3, and off thereafter.
 }}}
 On my systems, I get
 {{{
 MYSQL version : 10.1.34-MariaDB-0ubuntu0.18.04.1:
 $ mariadb --help | grep '^reconnect'
 reconnect                         TRUE
 or
 MYSQL version : 5.5.59-0ubuntu0.14.04.1:
 $ mysql --help | grep ^reconnect
 reconnect                         TRUE
 }}}
 The auto-reconnect feature is on by default on my installations.
 Nevertheless, I get this trace-back `MySQL server has gone away` in my
 test script, but mysql should automatically reconnect.
 That's bad. See below for a proposed solution.

 3) The `self.ping()` method of the class `LoggedCursor` is a weak
 reference, which seems to be a left-over from #12491
 See commit
 https://github.com/MythTV/mythtv/commit/eb622a4b71943ce16d2fb7623cbabccd863172c4
 of ticket #12491.

 Please correct me, if I am wrong.

 If I change these weak references to ping to a real references,
 I can ping now the MySQLdb connection with both methods, without loosing
 the connection to mysql.
 And, I can use the `LoggedCursor.ping()` method from outside without any
 problems.

 Conclusio:
 IMHO, the mentioned code snipped should look like this:

 {{{
 class LoggedCursor( MySQLdb.cursors.Cursor ):
     """
     Custom cursor, offering logging and error handling
     """
     def __init__(self, connection):
         super(LoggedCursor, self).__init__(connection)
         self.log = None
         self.ping = self._ping121                      ### XXX no weak
 refererence
         if  MySQLdb.version_info[:3] >= (1, 2, 2):     ### XXX correct
 comparison
             self.ping = self._ping122                  ### XXX no weak
 reference
         self.ping()

     def _ping121(self): self._get_db().ping()          ### XXX correct API
 of ping() for older versions
     def _ping122(self): self._get_db().ping(True)      ### XXX correct API
 of ping() for newer versions
 }}}



 Please look at my attached script and the result.

-- 
Ticket URL: <https://code.mythtv.org/trac/ticket/13320#comment:3>
MythTV <http://www.mythtv.org>
MythTV Media Center