Python bindings not reconnecting to database after a long pause.

[sir-maniac]

The python bindings aren't correctly checking for the version in _conn_mysqldb.py, as a result auto-reconnection is effectively disabled.

In the python MysqlDB library, auto-reconnection is default, until version 1.2.2, which requires a True be passed as an optional first parameter to the ping() function. Because the version detection is bugged, this parameter isn't being passed.

Currently the following check is used:

        if MySQLdb.version_info >= ('1','2','2'):
            self.ping = ref(self._ping122)

If you verify this in a python2.7 console it results in false, even though the version is newer:

>>> import MySQLdb
>>> MySQLdb.version_info >= ('1','2','2')
False
>>> print MySQLdb.version_info
(1, 3, 10, 'final', 0)

But the following code works:

>>> MySQLdb.version_info[:3] >= (1, 2, 2)
True

[sir-maniac]

I've created a pull request on github: ​https://github.com/MythTV/mythtv/pull/169

[sir-maniac]

Here's the patch for reference:

diff --git a/mythtv/bindings/python/MythTV/_conn_mysqldb.py b/mythtv/bindings/python/MythTV/_conn_mysqldb.py
index 676d147..ae47969 100644
--- a/mythtv/bindings/python/MythTV/_conn_mysqldb.py
+++ b/mythtv/bindings/python/MythTV/_conn_mysqldb.py
@@ -34,7 +34,7 @@
         super(LoggedCursor, self).__init__(connection)
         self.log = None
         self.ping = ref(self._ping121)
-        if MySQLdb.version_info >= ('1','2','2'):
+        if MySQLdb.version_info[:3] >= (1, 2, 2):
             self.ping = ref(self._ping122)
         self.ping()

[rcrdnalor]

Python testscript for checking timeout

[rcrdnalor]

Output of python test script for cheking tinouts

[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.

[rcrdnalor]

This is fixed by [9127e3436]

Fix mysql connection of the Python Bindings for python3.