Thread: [SQLObject] Regression in SQLObject 0.8.0b1
SQLObject is a Python ORM.
Brought to you by:
ianbicking,
phd
From: Markus G. <m.g...@gm...> - 2006-12-21 08:41:30
|
Hi, With SQLObject 0.8.0b1 my program hangs indefinitely with 100% CPU usage if the MySQL server has been gone (Error code 2006) while still holding the connection in SQLObject and performing a request afterwards. The relevant code is in mysqlconnection.py: def _executeRetry(self, conn, cursor, query): while 1: try: if self.need_unicode: # For MysqlDB 1.2.1 and later, we go # encoding->unicode->charset (in the mysql db) myquery = unicode(query, self.encoding) return cursor.execute(myquery) else: return cursor.execute(query) except MySQLdb.OperationalError, e: if e.args[0] in (2006, 20013): # SERVER_GONE or SERVER_LOST error if self.debug: self.printDebug(conn, str(e), 'ERROR') else: raise OperationalError(ErrorMessage(e)) * First, 20013 is a typo must be changed back to 2013. See http://dev.mysql.com/doc/refman/5.0/en/error-messages-client.html * Second, the newly introduced 2006 triggers the infinite loop in my case. I think there is a raise missing. The bug is already there in SQLObject 0.7.x but the error code 2006 wasn't checked there, so the buggy section which misses a raise was not entered. Kind regards, Markus |
From: Markus G. <m.g...@gm...> - 2006-12-28 17:56:14
|
Since there was no comment on the e-mail below, I resend it in case it got lost. ---- Hi, With SQLObject 0.8.0b1 my program hangs indefinitely with 100% CPU usage if the MySQL server has been gone (Error code 2006) while still holding the connection in SQLObject and performing a request afterwards. The relevant code is in mysqlconnection.py: def _executeRetry(self, conn, cursor, query): while 1: try: if self.need_unicode: # For MysqlDB 1.2.1 and later, we go # encoding->unicode->charset (in the mysql db) myquery = unicode(query, self.encoding) return cursor.execute(myquery) else: return cursor.execute(query) except MySQLdb.OperationalError, e: if e.args[0] in (2006, 20013): # SERVER_GONE or SERVER_LOST error if self.debug: self.printDebug(conn, str(e), 'ERROR') else: raise OperationalError(ErrorMessage(e)) * First, 20013 is a typo must be changed back to 2013. See http://dev.mysql.com/doc/refman/5.0/en/error-messages-client.html * Second, the newly introduced 2006 triggers the infinite loop in my case. I think there is a raise missing. The bug is already there in SQLObject 0.7.x but the error code 2006 wasn't checked there, so the buggy section which misses a raise was not entered. Kind regards, Markus |
From: Oleg B. <ph...@ph...> - 2006-12-28 18:35:48
|
On Thu, Dec 28, 2006 at 06:56:08PM +0100, Markus Gritsch wrote: > Since there was no comment on the e-mail below, I resend it in case it got > lost. Sorry for the silence. I have got it the first time. Just don't have time to work on it. There are other patches to test; I am working on the patch that implements delColumn and fromDatabase for SQLite. Regarding the patch. I think to fix the problem I will replace "while 1" with "for i in range(1000): sleep(1)". There should be no "raise", or the loop would be meaningless at all. The idea of the loop is to wait until the server is restarted. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Oleg B. <ph...@ph...> - 2007-01-16 15:28:59
|
On Tue, Jan 16, 2007 at 04:22:16PM +0100, Dr. Markus Gritsch wrote: > Sounds quite ok. And, BTW, there is an unexplored SolidConnection from DBUtil. I just remembered I have to look into it. > Still, have you any opinion about the pooling-stuff? It has to be preserved, I think. > Further, Dan had also some concerns about transactions. He wrote that > if the reconnectiong would happen at MySQLdb level, it could be > handled gracefully, but that things will get messed up which the patch > to SQLObject, if transactions are involved. Yes, I remember. This means we have to point everyone to the MySQLdb patch instead of implementing anything in SQLObject. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Oleg B. <ph...@ph...> - 2007-01-16 15:35:36
|
On Tue, Jan 16, 2007 at 06:28:53PM +0300, Oleg Broytmann wrote: > > Further, Dan had also some concerns about transactions. He wrote that > > if the reconnectiong would happen at MySQLdb level, it could be > > handled gracefully, but that things will get messed up which the patch > > to SQLObject, if transactions are involved. > > Yes, I remember. This means we have to point everyone to the MySQLdb > patch instead of implementing anything in SQLObject. Well, I see. There are two different problems - connection timeout and server restart. They have to be handled differently. Connection timeout has to be handled in the DB API driver. Server restart - in the SQLObject. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Dan P. <da...@ag...> - 2007-01-16 22:06:22
|
On Tuesday 16 January 2007 17:35, Oleg Broytmann wrote: > On Tue, Jan 16, 2007 at 06:28:53PM +0300, Oleg Broytmann wrote: > > > Further, Dan had also some concerns about transactions. He wrote > > > that if the reconnectiong would happen at MySQLdb level, it could > > > be handled gracefully, but that things will get messed up which the > > > patch to SQLObject, if transactions are involved. > > > > Yes, I remember. This means we have to point everyone to the > > MySQLdb patch instead of implementing anything in SQLObject. > > Well, I see. There are two different problems - connection timeout > and server restart. They have to be handled differently. Connection > timeout has to be handled in the DB API driver. Server restart - in the > SQLObject. IMO, I see no reason why a server restart cannot be handled by the same reconnect feature of the underlying mysql library. From the sqlobject point of view the situation is the same, no matter if the connection did timeout or the server was restarted. Once it timeouts a connection ceases to exist in the server much like when the server is restarted and looses all the connections. If the reconnect patch is applied to mysqldb, sqlobject needs no changes and shouldn't care about handling a restarted server (not that it can tell if the server died, was restarted or closed the connection because it was idle, anyway). In the end, probably the situation can be handled in sqlobject completely, if the transaction code takes care not to reconnect, but to raise an exception. Thus if no transactions are used, then the code can create a new connection everytime it detects a dead one, but if the connection is a transaction object in the middle of a transaction then it should just raise an error. However I fail to see the point to reimplement all this logic in sqlobject when it is already implemented by the mysql library. All that is needed is to make the reconnect option available to the python level and the solution is already out there, except that somebody insist on ignoring it for no good reason. -- Dan |
From: sophana <so...@zi...> - 2007-01-16 22:46:22
|
Dan Pascu a =E9crit : > IMO, I see no reason why a server restart cannot be handled by the same= =20 > reconnect feature of the underlying mysql library. From the sqlobject=20 > point of view the situation is the same, no matter if the connection di= d=20 > timeout or the server was restarted. Once it timeouts a connection ceas= es=20 > to exist in the server much like when the server is restarted and loose= s=20 > all the connections. > If the reconnect patch is applied to mysqldb, sqlobject needs no change= s=20 > and shouldn't care about handling a restarted server (not that it can=20 > tell if the server died, was restarted or closed the connection because= =20 > it was idle, anyway). > > In the end, probably the situation can be handled in sqlobject complete= ly,=20 > if the transaction code takes care not to reconnect, but to raise an=20 > exception. Thus if no transactions are used, then the code can create a= =20 > new connection everytime it detects a dead one, but if the connection i= s=20 > a transaction object in the middle of a transaction then it should just= =20 > raise an error. > > However I fail to see the point to reimplement all this logic in sqlobj= ect=20 > when it is already implemented by the mysql library. All that is needed= =20 > is to make the reconnect option available to the python level and the=20 > solution is already out there, except that somebody insist on ignoring = it=20 > for no good reason. > > =20 What I don't understand is that the mysql team choosed not to reconnect automatically in mysql5 because of transactions to be handled correctly. If this is handled correctly in the mysql API, why don't they just enable it by default? I don't see how the API can handle reconnection in a transaction... |
From: Oleg B. <ph...@ph...> - 2007-01-17 08:04:36
|
On Wed, Jan 17, 2007 at 12:06:11AM +0200, Dan Pascu wrote: > However I fail to see the point to reimplement all this logic in sqlobject > when it is already implemented by the mysql library. There are other backends beside MySQL. TurboGears people have reported they are having problems with PostgreSQL server restart. I don't know yet if psycopg can reconnect after the server restart. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Markus G. <m.g...@gm...> - 2007-03-06 17:59:43
|
On 3/6/07, Oleg Broytmann <ph...@ph...> wrote: > On Tue, Mar 06, 2007 at 02:59:11PM +0100, Markus Gritsch wrote: > > MySQLdb 1.2.2 has just been released on Saturday, and it allows > > setting the reconnect flag. I made some changes to mysqlconnection.py > > to support this. Please find the patch attached. > > Applied in the revisions 2396-2399 (0.7, 0.8, trunk, docs.) Thank you > very much! You're welcome :) > In the next round of patches I am going to change > if MySQLdb.version_info[0] > 1 or (MySQLdb.version_info[0] == 1 and... > to > if MySQLdb.version_info[:3] > (1, 2, 1) Ah, nice. Didn't know that works. There is a similar check in mysqlconnection.py around line 31. Maybe this should be rewritten using your more elegant syntax too. Kind regards, Markus |
From: Oleg B. <ph...@ph...> - 2007-03-06 18:17:45
|
On Tue, Mar 06, 2007 at 06:59:33PM +0100, Markus Gritsch wrote: > > if MySQLdb.version_info[:3] > (1, 2, 1) > > Ah, nice. Didn't know that works. Python is the best language! > There is a similar check in > mysqlconnection.py around line 31. Already fixed, of course. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Markus G. <m.g...@gm...> - 2006-12-28 18:50:25
|
On 12/28/06, Oleg Broytmann <ph...@ph...> wrote: > On Thu, Dec 28, 2006 at 06:56:08PM +0100, Markus Gritsch wrote: > > Since there was no comment on the e-mail below, I resend it in case it got > > lost. > > Sorry for the silence. I have got it the first time. Just don't have > time to work on it. ok, then sorry for resending it. > Regarding the patch. I think to fix the problem I will replace "while 1" > with "for i in range(1000): sleep(1)". There should be no "raise", or the > loop would be meaningless at all. The idea of the loop is to wait until the > server is restarted. Well, this would not solve the problem intoduced with SQLObject 0.8. An Error 2006 occurres in the following scenario using SQLObject 0.7: * My application is running, being connected to the MySQL database. * The MySQL service is stopped and restarted. * Every query from now on on the already made connection fails with the Error 2006. * The user can see this in a message box, and reconnect to the database which solves the problem. So the Error 2006 occurrs if the connection to the old server was broken. It does not matter that the MySQL server is already being restarted. Trying it 1000 times as mentioned above would not fix the problem. It would even make it worse, since the user gets no feedback about what is wrong. Just a hanging app. Kind regards, Markus |
From: Ian B. <ia...@co...> - 2006-12-28 18:53:10
|
Markus Gritsch wrote: > On 12/28/06, Oleg Broytmann <ph...@ph...> wrote: >> On Thu, Dec 28, 2006 at 06:56:08PM +0100, Markus Gritsch wrote: >>> Since there was no comment on the e-mail below, I resend it in case it got >>> lost. >> Sorry for the silence. I have got it the first time. Just don't have >> time to work on it. > > ok, then sorry for resending it. > >> Regarding the patch. I think to fix the problem I will replace "while 1" >> with "for i in range(1000): sleep(1)". There should be no "raise", or the >> loop would be meaningless at all. The idea of the loop is to wait until the >> server is restarted. > > Well, this would not solve the problem intoduced with SQLObject 0.8. > An Error 2006 occurres in the following scenario using SQLObject 0.7: > * My application is running, being connected to the MySQL database. > * The MySQL service is stopped and restarted. > * Every query from now on on the already made connection fails with > the Error 2006. > * The user can see this in a message box, and reconnect to the > database which solves the problem. > > So the Error 2006 occurrs if the connection to the old server was > broken. It does not matter that the MySQL server is already being > restarted. Trying it 1000 times as mentioned above would not fix the > problem. It would even make it worse, since the user gets no feedback > about what is wrong. Just a hanging app. We should really figure out a way to use DBUtils under SQLObject in some way, as it would solve this problem: http://www.webwareforpython.org/DBUtils -- Ian Bicking | ia...@co... | http://blog.ianbicking.org |
From: Oleg B. <ph...@ph...> - 2006-12-28 19:23:34
|
On Thu, Dec 28, 2006 at 12:52:57PM -0600, Ian Bicking wrote: > We should really figure out a way to use DBUtils under SQLObject in some > way, as it would solve this problem: > > http://www.webwareforpython.org/DBUtils "DBUtils is a suite of Python modules allowing to connect in a safe and efficient way between a threaded Python application and a database." I very much hope it doesn't require multithreading, only allows it! Perhaps it has to be used it the DBAPI class, at the lowest level of SQLObject connection. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Oleg B. <ph...@ph...> - 2006-12-28 18:57:33
|
On Thu, Dec 28, 2006 at 07:50:23PM +0100, Markus Gritsch wrote: > Well, this would not solve the problem intoduced with SQLObject 0.8. > An Error 2006 occurres in the following scenario using SQLObject 0.7: > * My application is running, being connected to the MySQL database. > * The MySQL service is stopped and restarted. > * Every query from now on on the already made connection fails with > the Error 2006. So the solution for now, I think, would be to stop testing for the error 2006 and raise an error. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Dan P. <da...@ag...> - 2006-12-29 07:57:03
|
On Thursday 28 December 2006 20:58, Oleg Broytmann wrote: > On Thu, Dec 28, 2006 at 07:50:23PM +0100, Markus Gritsch wrote: > > Well, this would not solve the problem intoduced with SQLObject 0.8. > > An Error 2006 occurres in the following scenario using SQLObject 0.7: > > * My application is running, being connected to the MySQL database. > > * The MySQL service is stopped and restarted. > > * Every query from now on on the already made connection fails with > > the Error 2006. > > So the solution for now, I think, would be to stop testing for the > error 2006 and raise an error. Please don't do this. 2006 is returned more often than 2013 (read the comments in the patch I submitted). The problem Markus has is that his mysqldb module will never reconnect because it doesn't set the reconnect flag. This is why he gets 2006 over and over. Please consider the patch I submitted as it fixes the problem both for those that use the original mysqldb module (that doesn't reconnect) and for people who use a modified mysqldb module that does reconnect. With your change the latter group will no longer be able to reconnect as 2006 is raised first and is no longer handled. -- Dan |
From: Markus G. <m.g...@gm...> - 2006-12-28 19:01:10
|
On 12/28/06, Oleg Broytmann <ph...@ph...> wrote: > So the solution for now, I think, would be to stop testing for the > error 2006 and raise an error. Yes, this would be the solution with the smallest effort. It would result in the same code as in SQLObject 0.7. As already mentioned in the initial e-mail, the type 20013 must also be changed back to 2013. Kind regards, Markus |
From: Oleg B. <ph...@ph...> - 2006-12-28 19:15:09
|
On Thu, Dec 28, 2006 at 08:01:06PM +0100, Markus Gritsch wrote: > On 12/28/06, Oleg Broytmann <ph...@ph...> wrote: > > So the solution for now, I think, would be to stop testing for the > >error 2006 and raise an error. > > Yes, this would be the solution with the smallest effort. It would > result in the same code as in SQLObject 0.7. > > As already mentioned in the initial e-mail, the type 20013 must also > be changed back to 2013. Ok, will fix this. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Oleg B. <ph...@ph...> - 2006-12-28 20:21:24
|
On Thu, Dec 28, 2006 at 08:01:06PM +0100, Markus Gritsch wrote: > On 12/28/06, Oleg Broytmann <ph...@ph...> wrote: > > > So the solution for now, I think, would be to stop testing for the > >error 2006 and raise an error. > > Yes, this would be the solution with the smallest effort. It would > result in the same code as in SQLObject 0.7. > > As already mentioned in the initial e-mail, the type 20013 must also > be changed back to 2013. Fixed in the trunk and 0.8 branch (revisions 2158-2160). Will be in 0.8.0b2. Thank you! Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Dan P. <da...@ag...> - 2006-12-29 07:45:50
Attachments:
sqlobject-mysql-reconnect-fix.patch
|
On Thursday 28 December 2006 20:36, Oleg Broytmann wrote: > On Thu, Dec 28, 2006 at 06:56:08PM +0100, Markus Gritsch wrote: > > Since there was no comment on the e-mail below, I resend it in case > > it got lost. > > Sorry for the silence. I have got it the first time. Just don't have > time to work on it. There are other patches to test; I am working on > the patch that implements delColumn and fromDatabase for SQLite. > Regarding the patch. I think to fix the problem I will replace > "while 1" with "for i in range(1000): sleep(1)". There should be no > "raise", or the loop would be meaningless at all. The idea of the loop > is to wait until the server is restarted. I disagree with you here. It's a bad design to let SQLObject (a library) make this decision (what to do and how long to wait when a connection is lost and cannot be reestablished). I agree that SQLObject should attempt to reconnect, for cases where the server was restarted or the connection was lost because it was idle too long. But if it can't reconnect immediately it should signal this error to the caller (application). This reconnect is only to avoid extra hassle in the application for something that can be done automatically and takes no extra time. Trying to reconnect forever or for long periods of time is a bad thing. Just imagine the scenario where I have a realtime application using sqlobject and a replicated master-slave database. If the master dies, or is taken out for maintenance, I want the application to commute to the slave database as quickly as possible (as soon as it finds out that the connection was lost). With the original code or your proposal I cannot do this and my application will block waiting for sqlobject to do something it cannot do because it has no knowledge of the situation or what the application intends to do in such cases. Attached is a patch that fixes this in a better way. It still allows reconnects if the database connection was lost because of a server restart or the connection being closed because it was idle too long, but will allow the application to make decisions on its own if the database is down for good and cannot be reconnected. -- Dan |
From: Dan P. <da...@ag...> - 2006-12-29 07:50:38
|
On Thursday 28 December 2006 20:50, Markus Gritsch wrote: > Well, this would not solve the problem intoduced with SQLObject 0.8. > An Error 2006 occurres in the following scenario using SQLObject 0.7: > * My application is running, being connected to the MySQL database. > * The MySQL service is stopped and restarted. > * Every query from now on on the already made connection fails with > the Error 2006. > * The user can see this in a message box, and reconnect to the > database which solves the problem. > > So the Error 2006 occurrs if the connection to the old server was > broken. It does not matter that the MySQL server is already being > restarted. Trying it 1000 times as mentioned above would not fix the > problem. It would even make it worse, since the user gets no feedback > about what is wrong. Just a hanging app. Your problem is that you use a 5.0.x (or newer server) and it no longer permits autoreconnecting by default. The client must set a reconnect flag when making the connection to signal that it desires this behavior and the python mysqldb module doesn't set this flag. If you use debian, I submitted a patch to them and the debian python-mysqldb does reconnect when a connection is lost. Else you can patch your python-mysqldb module to set this flag, or you can complain to the python-mysqldb authors to find a solution for allowing this reconnect flag to be set if needed. -- Dan |
From: Markus G. <m.g...@gm...> - 2006-12-29 17:10:40
|
On 12/29/06, Dan Pascu <da...@ag...> wrote: > Your problem is that you use a 5.0.x (or newer server) and it no longer > permits autoreconnecting by default. The client must set a reconnect flag > when making the connection to signal that it desires this behavior and > the python mysqldb module doesn't set this flag. If you use debian, I > submitted a patch to them and the debian python-mysqldb does reconnect > when a connection is lost. > > Else you can patch your python-mysqldb module to set this flag, or you can > complain to the python-mysqldb authors to find a solution for allowing > this reconnect flag to be set if needed. Thank you for the detailed explanation. I saw that there is already a patch to MySQLdb available which unfortunately is still not applied: http://sourceforge.net/tracker/index.php?func=detail&aid=1483074&group_id=22307&atid=374934 I hope it will be included in the next release... Kind regards, Markus |
From: Oleg B. <ph...@ph...> - 2006-12-29 08:01:11
|
On Fri, Dec 29, 2006 at 09:45:35AM +0200, Dan Pascu wrote: > Trying to reconnect forever or for long periods of time is a bad thing. Agree. > Attached is a patch that fixes this in a better way. It still allows > reconnects if the database connection was lost because of a server > restart or the connection being closed because it was idle too long, but > will allow the application to make decisions on its own if the database > is down for good and cannot be reconnected. Thank you! > + for c in range(0, 3): > try: > if self.need_unicode: > # For MysqlDB 1.2.1 and later, we go > @@ -88,7 +100,9 @@ > else: > return cursor.execute(query) > except MySQLdb.OperationalError, e: > - if e.args[0] == 2013: # SERVER_LOST error > + if e.args[0] in (2006, 2013): # SERVER_GONE or SERVER_LOST error > + if c == 2: > + raise OperationalError(ErrorMessage(e)) I think I can move this "if...raise" out of the loop to "else" clause. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Dan P. <da...@ag...> - 2006-12-29 08:13:36
|
On Friday 29 December 2006 10:02, Oleg Broytmann wrote: > > + for c in range(0, 3): > > try: > > if self.need_unicode: > > # For MysqlDB 1.2.1 and later, we go > > @@ -88,7 +100,9 @@ > > else: > > return cursor.execute(query) > > except MySQLdb.OperationalError, e: > > - if e.args[0] == 2013: # SERVER_LOST error > > + if e.args[0] in (2006, 2013): # SERVER_GONE or > > SERVER_LOST error + if c == 2: > > + raise OperationalError(ErrorMessage(e)) > > I think I can move this "if...raise" out of the loop to "else" > clause. I was thinking of that, but wasn't sure if in the else clause the exception variable (e) is visible to be raised, so I took the safer approach. But if you know for sure it is visible in that context, than I agree it's more elegant to write it that way. And BTW, the if c==2 is no longer needed in the else clause. Only a simple raise. -- Dan |
From: Oleg B. <ph...@ph...> - 2006-12-29 15:19:33
|
On Fri, Dec 29, 2006 at 09:45:35AM +0200, Dan Pascu wrote: > Attached is a patch that fixes this in a better way. It still allows > reconnects if the database connection was lost because of a server > restart or the connection being closed because it was idle too long, but > will allow the application to make decisions on its own if the database > is down for good and cannot be reconnected. > Index: mysqlconnection.py I applied it verbatim, revisions 2166-2168. Thank you! Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Dan P. <da...@ag...> - 2006-12-30 05:52:11
|
On Friday 29 December 2006 17:20, Oleg Broytmann wrote: > On Fri, Dec 29, 2006 at 09:45:35AM +0200, Dan Pascu wrote: > > Attached is a patch that fixes this in a better way. It still allows > > reconnects if the database connection was lost because of a server > > restart or the connection being closed because it was idle too long, > > but will allow the application to make decisions on its own if the > > database is down for good and cannot be reconnected. > > > > Index: mysqlconnection.py > > I applied it verbatim, revisions 2166-2168. Thank you! What changed your mind about moving the exception raising to the else clause? -- Dan |