openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #15949
[Merge] lp:~dave-warnock/openlp/dbclean into lp:openlp
Dave Warnock has proposed merging lp:~dave-warnock/openlp/dbclean into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~dave-warnock/openlp/dbclean/+merge/109546
This update is a tidy up of the code that fixed bug #927473
Changes are made due to the following issues:
- the code assumed that exception OperationalError would only be thrown by
mySQL temporarily disappearing. However, other dbms can throw this
exception and usually for errors that mean a retry will also fail.
- the code repeated the actual code of the method within the exception
handler. This means code duplication and also that any new exceptions
are not handled by the same exception handler so for example their
transaction will not get rolled back.
- not all potential dbms exceptions were caught and so in some cases the
database transaction was not rolled back
The solution is to retry transactions where an OperationalError is thrown.
Currently these are retried upto 3 times before the error is logged and
the method returns false. By retrying the transaction we ensure that the
same transaction code with the same exception handlers is used each time.
An additional catchall exception has been added where there is a transaction
to ensure that it is rolled back. As with the OperationError this throws
the exception back up the stack.
It has been tested only on SQLite. I have tested manually raising the various
exceptions to check the retries worked.
--
https://code.launchpad.net/~dave-warnock/openlp/dbclean/+merge/109546
Your team OpenLP Core is requested to review the proposed merge of lp:~dave-warnock/openlp/dbclean into lp:openlp.
=== modified file 'openlp/core/lib/db.py'
--- openlp/core/lib/db.py 2012-03-12 19:35:29 +0000
+++ openlp/core/lib/db.py 2012-06-10 21:56:18 +0000
@@ -238,27 +238,30 @@
``commit``
Commit the session with this object
"""
- try:
- self.session.add(object_instance)
- if commit:
- self.session.commit()
- self.is_dirty = True
- return True
- except OperationalError:
- # This exception clause is for users running MySQL which likes
- # to terminate connections on its own without telling anyone.
- # See bug #927473
- log.exception(u'Probably a MySQL issue - "MySQL has gone away"')
- self.session.rollback()
- self.session.add(object_instance)
- if commit:
- self.session.commit()
- self.is_dirty = True
- return True
- except InvalidRequestError:
- self.session.rollback()
- log.exception(u'Object save failed')
- return False
+ for tryCount in range(3):
+ try:
+ self.session.add(object_instance)
+ if commit:
+ self.session.commit()
+ self.is_dirty = True
+ return True
+ except OperationalError:
+ # This exception clause is for users running MySQL which likes
+ # to terminate connections on its own without telling anyone.
+ # See bug #927473
+ # However, other dbms can raise it, usually in a non-recoverable
+ # way. So we only retry 3 times.
+ log.exception(u'Probably a MySQL issue - "MySQL has gone away"')
+ self.session.rollback()
+ if tryCount >= 2:
+ raise
+ except InvalidRequestError:
+ self.session.rollback()
+ log.exception(u'Object list save failed')
+ return False
+ except:
+ self.session.rollback()
+ raise
def save_objects(self, object_list, commit=True):
"""
@@ -270,27 +273,30 @@
``commit``
Commit the session with this object
"""
- try:
- self.session.add_all(object_list)
- if commit:
- self.session.commit()
- self.is_dirty = True
- return True
- except OperationalError:
- # This exception clause is for users running MySQL which likes
- # to terminate connections on its own without telling anyone.
- # See bug #927473
- log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
- self.session.rollback()
- self.session.add_all(object_list)
- if commit:
- self.session.commit()
- self.is_dirty = True
- return True
- except InvalidRequestError:
- self.session.rollback()
- log.exception(u'Object list save failed')
- return False
+ for tryCount in range(3):
+ try:
+ self.session.add_all(object_list)
+ if commit:
+ self.session.commit()
+ self.is_dirty = True
+ return True
+ except OperationalError:
+ # This exception clause is for users running MySQL which likes
+ # to terminate connections on its own without telling anyone.
+ # See bug #927473
+ # However, other dbms can raise it, usually in a non-recoverable
+ # way. So we only retry 3 times.
+ log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
+ self.session.rollback()
+ if tryCount >= 2:
+ raise
+ except InvalidRequestError:
+ self.session.rollback()
+ log.exception(u'Object list save failed')
+ return False
+ except:
+ self.session.rollback()
+ raise
def get_object(self, object_class, key=None):
"""
@@ -305,15 +311,18 @@
if not key:
return object_class()
else:
- try:
- return self.session.query(object_class).get(key)
- except OperationalError:
- # This exception clause is for users running MySQL which likes
- # to terminate connections on its own without telling anyone.
- # See bug #927473
- log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
- self.session.rollback()
- return self.session.query(object_class).get(key)
+ for tryCount in range(3):
+ try:
+ return self.session.query(object_class).get(key)
+ except OperationalError:
+ # This exception clause is for users running MySQL which likes
+ # to terminate connections on its own without telling anyone.
+ # See bug #927473
+ # However, other dbms can raise it, usually in a non-recoverable
+ # way. So we only retry 3 times.
+ log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
+ if tryCount >= 2:
+ raise
def get_object_filtered(self, object_class, filter_clause):
"""
@@ -325,15 +334,18 @@
``filter_clause``
The criteria to select the object by
"""
- try:
- return self.session.query(object_class).filter(filter_clause).first()
- except OperationalError:
- # This exception clause is for users running MySQL which likes
- # to terminate connections on its own without telling anyone.
- # See bug #927473
- log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
- self.session.rollback()
- return self.session.query(object_class).filter(filter_clause).first()
+ for tryCount in range(3):
+ try:
+ return self.session.query(object_class).filter(filter_clause).first()
+ except OperationalError:
+ # This exception clause is for users running MySQL which likes
+ # to terminate connections on its own without telling anyone.
+ # See bug #927473
+ # However, other dbms can raise it, usually in a non-recoverable
+ # way. So we only retry 3 times.
+ log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
+ if tryCount >= 2:
+ raise
def get_all_objects(self, object_class, filter_clause=None,
order_by_ref=None):
@@ -357,15 +369,18 @@
query = query.order_by(*order_by_ref)
elif order_by_ref is not None:
query = query.order_by(order_by_ref)
- try:
- return query.all()
- except OperationalError:
- # This exception clause is for users running MySQL which likes
- # to terminate connections on its own without telling anyone.
- # See bug #927473
- log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
- self.session.rollback()
- return query.all()
+ for tryCount in range(3):
+ try:
+ return query.all()
+ except OperationalError:
+ # This exception clause is for users running MySQL which likes
+ # to terminate connections on its own without telling anyone.
+ # See bug #927473
+ # However, other dbms can raise it, usually in a non-recoverable
+ # way. So we only retry 3 times.
+ log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
+ if tryCount >= 2:
+ raise
def get_object_count(self, object_class, filter_clause=None):
"""
@@ -381,15 +396,18 @@
query = self.session.query(object_class)
if filter_clause is not None:
query = query.filter(filter_clause)
- try:
- return query.count()
- except OperationalError:
- # This exception clause is for users running MySQL which likes
- # to terminate connections on its own without telling anyone.
- # See bug #927473
- log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
- self.session.rollback()
- return query.count()
+ for tryCount in range(3):
+ try:
+ return query.count()
+ except OperationalError:
+ # This exception clause is for users running MySQL which likes
+ # to terminate connections on its own without telling anyone.
+ # See bug #927473
+ # However, other dbms can raise it, usually in a non-recoverable
+ # way. So we only retry 3 times.
+ log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
+ if tryCount >= 2:
+ raise
def delete_object(self, object_class, key):
"""
@@ -403,25 +421,29 @@
"""
if key != 0:
object_instance = self.get_object(object_class, key)
- try:
- self.session.delete(object_instance)
- self.session.commit()
- self.is_dirty = True
- return True
- except OperationalError:
- # This exception clause is for users running MySQL which likes
- # to terminate connections on its own without telling anyone.
- # See bug #927473
- log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
- self.session.rollback()
- self.session.delete(object_instance)
- self.session.commit()
- self.is_dirty = True
- return True
- except InvalidRequestError:
- self.session.rollback()
- log.exception(u'Failed to delete object')
- return False
+ for tryCount in range(3):
+ try:
+ self.session.delete(object_instance)
+ self.session.commit()
+ self.is_dirty = True
+ return True
+ except OperationalError:
+ # This exception clause is for users running MySQL which likes
+ # to terminate connections on its own without telling anyone.
+ # See bug #927473
+ # However, other dbms can raise it, usually in a non-recoverable
+ # way. So we only retry 3 times.
+ log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
+ self.session.rollback()
+ if tryCount >= 2:
+ raise
+ except InvalidRequestError:
+ self.session.rollback()
+ log.exception(u'Failed to delete object')
+ return False
+ except:
+ self.session.rollback()
+ raise
else:
return True
@@ -439,31 +461,32 @@
The filter governing selection of objects to return. Defaults to
None.
"""
- try:
- query = self.session.query(object_class)
- if filter_clause is not None:
- query = query.filter(filter_clause)
- query.delete(synchronize_session=False)
- self.session.commit()
- self.is_dirty = True
- return True
- except OperationalError:
- # This exception clause is for users running MySQL which likes
- # to terminate connections on its own without telling anyone.
- # See bug #927473
- log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
- self.session.rollback()
- query = self.session.query(object_class)
- if filter_clause is not None:
- query = query.filter(filter_clause)
- query.delete(synchronize_session=False)
- self.session.commit()
- self.is_dirty = True
- return True
- except InvalidRequestError:
- self.session.rollback()
- log.exception(u'Failed to delete %s records', object_class.__name__)
- return False
+ for tryCount in range(3):
+ try:
+ query = self.session.query(object_class)
+ if filter_clause is not None:
+ query = query.filter(filter_clause)
+ query.delete(synchronize_session=False)
+ self.session.commit()
+ self.is_dirty = True
+ return True
+ except OperationalError:
+ # This exception clause is for users running MySQL which likes
+ # to terminate connections on its own without telling anyone.
+ # See bug #927473
+ # However, other dbms can raise it, usually in a non-recoverable
+ # way. So we only retry 3 times.
+ log.exception(u'Probably a MySQL issue, "MySQL has gone away"')
+ self.session.rollback()
+ if tryCount >= 2:
+ raise
+ except InvalidRequestError:
+ self.session.rollback()
+ log.exception(u'Failed to delete %s records', object_class.__name__)
+ return False
+ except:
+ self.session.rollback()
+ raise
def finalise(self):
"""
Follow ups
