From: Jelte Jansen <jelte@isc.org>
Date: Wed, 12 Dec 2012 10:30:56 +0000 (+0100)
Subject: [2541] Catch and log exception in transaction rollback
X-Git-Tag: bind10-1.0.0-beta-release~26^2~9
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=eb65b11ca44fb2b2c128b317449272240ce337c3;p=thirdparty%2Fkea.git

[2541] Catch and log exception in transaction rollback
---

diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index b83c4011b7..62da2e0d44 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -55,7 +55,18 @@ public:
     }
     ~TransactionHolder() {
         if (!committed_) {
-            accessor_.rollback();
+            try {
+                accessor_.rollback();
+            } catch (const DataSourceError& e) {
+                // We generally expect that rollback always succeeds, and
+                // it should in fact succeed in a way we execute it.  But
+                // as the public API allows rollback() to fail and
+                // throw, we should expect it.  Obviously we cannot re-throw
+                // it.  The best we can do is to log it as a critical error.
+                logger.error(DATASRC_DATABASE_TRANSACTION_ROLLBACKFAIL).
+                            arg(accessor_.getDBName()).
+                            arg(e.what());
+            }
         }
     }
     void commit() {
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index 84373315b6..9db8a8fa66 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -186,8 +186,8 @@ public:
     /// responsibility of the caller to do so.
     ///
     /// Callers must also start a transaction before calling this method,
-    /// implementations may throw DataSourceError if this has not been done.
-    /// Callers should also expect DataSourceErrors for other potential
+    /// implementations should throw DataSourceError if this has not been
+    /// done. Callers should also expect DataSourceErrors for other potential
     /// problems.
     ///
     /// \param name The (fully qualified) domain name of the zone to add.
@@ -1399,9 +1399,6 @@ public:
     /// does not, creates it, commits, and returns true. If the zone
     /// does exist already, it does nothing (except abort the transaction)
     /// and returns false.
-    ///
-    /// \param name The (fully qualified) name of the zone to create
-    /// \return True if the zone was added, false if it already existed
     virtual bool createZone(const isc::dns::Name& name);
 
     /// \brief Get the zone iterator
diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes
index e7cb9d3f9d..e9b4c908af 100644
--- a/src/lib/datasrc/datasrc_messages.mes
+++ b/src/lib/datasrc/datasrc_messages.mes
@@ -216,6 +216,16 @@ to find any invalid data and fix it.
 No match (not even a wildcard) was found in the named data source for the given
 name/type/class in the data source.
 
+% DATASRC_DATABASE_TRANSACTION_ROLLBACKFAIL failed to roll back transaction on %1: %2
+A transaction on the database was rolled back without committing the
+changes to the database, but the rollback itself unexpectedly fails.
+The higher level implementation does not expect it to fail, so this means
+either a serious operational error in the underlying data source (such as a
+system failure of a database) or software bug in the underlying data source
+implementation.  In either case if this message is logged the administrator
+should carefully examine the underlying data source to see what exactly
+happens and whether the data is still valid.
+
 % DATASRC_DATABASE_UPDATER_COMMIT updates committed for '%1/%2' on %3
 Debug information.  A set of updates to a zone has been successfully
 committed to the corresponding database backend.  The zone name,