Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When use pymsql driver, the setsession params for PooledDB is not work after mysql server restart #23

Closed
salmon7 opened this issue Dec 13, 2019 · 3 comments

Comments

@salmon7
Copy link

salmon7 commented Dec 13, 2019

I meet something strange in production. I have set setsession=["set autocommit=1"] in PooledDB and think it will be auto commit even if the connection has been reconnect.

But it seems the setsession do not work for pymysql in reconnect situation. Here is my demo.

# -*- coding: utf-8 -*-

import pymysql
import time
import traceback
from DBUtils.PooledDB import PooledDB
from pymysql import MySQLError

pymysql.install_as_MySQLdb()
con = None
pooledDB = None

def try_insert():
    i = 0
    while i < 60:
        print "============ {0} ============".format(i)
        con = pooledDB.connection()
        try:
            cursor = con.cursor(pymysql.cursors.DictCursor)
            if not cursor:
                print "cursor is {0}".format(cursor)
            select_sql = "insert into user2(name,age) values('zhang', 20)"
            ret_rows = cursor.execute(select_sql)
            print cursor._last_executed
            print "ret_rows is {0}".format(ret_rows)

        except MySQLError as e:
            print "MySQLError error: {0}".format(e)
            print traceback.format_exc()
        except Exception as e:
            print "Exception error: {0}".format(e)
            print traceback.format_exc()
    
        i = i +1 
        time.sleep(1)
        con.close()


if __name__ == "__main__":
    db_conf = {'user':'root','passwd':'test','host':'127.0.0.1','port':3306,'connect_timeout':5,'db':'test_dbutils'}

    pooledDB = PooledDB(
        creator=pymysql,  
        maxconnections=4,  
        mincached=0,  
        maxcached=0,  
        maxshared=0,  
        blocking=True,  
        maxusage=None,  
        setsession=["set autocommit=1"],  
        ping=1,  
        reset=False,  
        **db_conf
    )
    
    try_insert()

When the demo run, try to restart mysql server, and after the application reconnects mysql successfully and then get no error return, but, actually, it will not insert to mysql for rest sql. After debug the source code I think the reason it that in this situation it reconnects by pymysql directly not by dbutils.

I add some debug code in SteadyDBConnection to help me understand the process.

    def _ping_check(self, ping=1, reconnect=True):
        """Check whether the connection is still alive using ping().

        If the the underlying connection is not active and the ping
        parameter is set accordingly, the connection will be recreated
        unless the connection is currently inside a transaction.
        """
        if ping & self._ping:
            try: # if possible, ping the connection
                my_reconnect=True
                print "try to ping by pymysql(reconnect={0})".format(my_reconnect)
                alive = self._con.ping(reconnect=my_reconnect)
                # alive = self._con.ping()
            except (AttributeError, IndexError, TypeError, ValueError):
                print "ping method is not available"
                self._ping = 0 # ping() is not available
                alive = None
                reconnect = False
            except Exception,e :
                print "try to ping by pymysql(reconnect={0}) fail".format(my_reconnect)
                alive = False
            else:
                if alive is None:
                    alive = True
                if alive:
                    reconnect = False
                print "try to ping by pymysql(reconnect={0}) success".format(my_reconnect)
            if reconnect and not self._transaction:
                try: # try to reopen the connection
                    print "try to reconnect by dbutils"
                    con = self._create()
                except Exception:
                    print "try to reconnect by dbutils fail"
                    pass
                else:
                    print "try to reconnect by dbutils success"
                    self._close()
                    self._store(con)
                    alive = True
            return alive

For pymysql, its ping method has default reconnect behavior, and the my_reconnect variable addde for debug always will be true. It seems that dbutils assumed ping will not reconnect. So the setsession parameter will not apply to pymysql reconnect situation. Should setsession used in this way? Is it possible to add compatible parameter for driver which has reconnect parameter in its ping method?

Actually, I found another way to solve my promble by adding autocommit in db_con, like

    db_conf = {'user':'root','passwd':'test','host':'127.0.0.1','port':3306,'connect_timeout':5,'db':'test_dbutils',"autocommit":True}

My local python and package version:
dbutils : 1.1
pymysql : 0.9.3
Python : 2.7.13

@Cito
Copy link
Member

Cito commented Dec 14, 2019

Thanks for the detailed analysis which I think is correct.

I also recommend to add autocommit to the connect parameters instead of using setsession.

But nevertheless, I think this should be fixed, for other uses of setsession with pymysql.

My suggested fix would be to replace self._con.ping() in _ping_check() with the following:

        try:
            self._con.ping(False)  # do not reconnect
        except TypeError:  # reconnect parameter not supported
            self._con.ping()

This should support pymysql, and other drivers which do not have a reconnect argument.

Can you check if this works for you? I would then add this in the next version of DBUtils.

@salmon7
Copy link
Author

salmon7 commented Dec 16, 2019

It should be self._con.ping(False) # do not reconnect, which delete extra _. I have try it and it work for me. Thank you very much~
👍

@Cito
Copy link
Member

Cito commented Dec 16, 2019

Sorry, yes, the _ was a typo (updated already). Good to hear this solves the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants