From 913b0433f60df96d92d7957846bbf85b08d8ccf0 Mon Sep 17 00:00:00 2001 From: Dave Warnock Date: Sun, 10 Jun 2012 22:45:02 +0100 Subject: [PATCH 1/2] 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 up to 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. --- openlp/core/lib/db.py | 267 +++++++++++++++++++++++------------------- 1 file changed, 145 insertions(+), 122 deletions(-) diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index e097983eb..8f24288ba 100644 --- a/openlp/core/lib/db.py +++ b/openlp/core/lib/db.py @@ -238,27 +238,30 @@ class Manager(object): ``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 @@ class Manager(object): ``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 @@ class Manager(object): 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 @@ class Manager(object): ``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 @@ class Manager(object): 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 @@ class Manager(object): 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 @@ class Manager(object): """ 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 @@ class Manager(object): 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): """ From b9b0d83a00a1d99d7605fdc5f71fe73912a7d86d Mon Sep 17 00:00:00 2001 From: Dave Warnock Date: Mon, 11 Jun 2012 15:11:53 +0100 Subject: [PATCH 2/2] renamed tryCount local variable to try_count --- openlp/core/lib/db.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index 8f24288ba..e4cbcea0f 100644 --- a/openlp/core/lib/db.py +++ b/openlp/core/lib/db.py @@ -238,7 +238,7 @@ class Manager(object): ``commit`` Commit the session with this object """ - for tryCount in range(3): + for try_count in range(3): try: self.session.add(object_instance) if commit: @@ -253,7 +253,7 @@ class Manager(object): # way. So we only retry 3 times. log.exception(u'Probably a MySQL issue - "MySQL has gone away"') self.session.rollback() - if tryCount >= 2: + if try_count >= 2: raise except InvalidRequestError: self.session.rollback() @@ -273,7 +273,7 @@ class Manager(object): ``commit`` Commit the session with this object """ - for tryCount in range(3): + for try_count in range(3): try: self.session.add_all(object_list) if commit: @@ -288,7 +288,7 @@ class Manager(object): # way. So we only retry 3 times. log.exception(u'Probably a MySQL issue, "MySQL has gone away"') self.session.rollback() - if tryCount >= 2: + if try_count >= 2: raise except InvalidRequestError: self.session.rollback() @@ -311,7 +311,7 @@ class Manager(object): if not key: return object_class() else: - for tryCount in range(3): + for try_count in range(3): try: return self.session.query(object_class).get(key) except OperationalError: @@ -321,7 +321,7 @@ class Manager(object): # 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: + if try_count >= 2: raise def get_object_filtered(self, object_class, filter_clause): @@ -334,7 +334,7 @@ class Manager(object): ``filter_clause`` The criteria to select the object by """ - for tryCount in range(3): + for try_count in range(3): try: return self.session.query(object_class).filter(filter_clause).first() except OperationalError: @@ -344,7 +344,7 @@ class Manager(object): # 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: + if try_count >= 2: raise def get_all_objects(self, object_class, filter_clause=None, @@ -369,7 +369,7 @@ class Manager(object): query = query.order_by(*order_by_ref) elif order_by_ref is not None: query = query.order_by(order_by_ref) - for tryCount in range(3): + for try_count in range(3): try: return query.all() except OperationalError: @@ -379,7 +379,7 @@ class Manager(object): # 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: + if try_count >= 2: raise def get_object_count(self, object_class, filter_clause=None): @@ -396,7 +396,7 @@ class Manager(object): query = self.session.query(object_class) if filter_clause is not None: query = query.filter(filter_clause) - for tryCount in range(3): + for try_count in range(3): try: return query.count() except OperationalError: @@ -406,7 +406,7 @@ class Manager(object): # 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: + if try_count >= 2: raise def delete_object(self, object_class, key): @@ -421,7 +421,7 @@ class Manager(object): """ if key != 0: object_instance = self.get_object(object_class, key) - for tryCount in range(3): + for try_count in range(3): try: self.session.delete(object_instance) self.session.commit() @@ -435,7 +435,7 @@ class Manager(object): # way. So we only retry 3 times. log.exception(u'Probably a MySQL issue, "MySQL has gone away"') self.session.rollback() - if tryCount >= 2: + if try_count >= 2: raise except InvalidRequestError: self.session.rollback() @@ -461,7 +461,7 @@ class Manager(object): The filter governing selection of objects to return. Defaults to None. """ - for tryCount in range(3): + for try_count in range(3): try: query = self.session.query(object_class) if filter_clause is not None: @@ -478,7 +478,7 @@ class Manager(object): # way. So we only retry 3 times. log.exception(u'Probably a MySQL issue, "MySQL has gone away"') self.session.rollback() - if tryCount >= 2: + if try_count >= 2: raise except InvalidRequestError: self.session.rollback()