BIND 10 trac1068review2, updated. fcb2409598d37e2078076cf43794ef6c445ac22f [1068] cleanup
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Aug 30 05:44:43 UTC 2011
The branch, trac1068review2 has been updated
via fcb2409598d37e2078076cf43794ef6c445ac22f (commit)
via da8bfe82aa18a67b1a99fa459f48cea89ee2a41a (commit)
via 7980a6c8e598d34f5f733f5c6c3ca83c0a0f1187 (commit)
from 9c62a36b0ebf9ff4ef3dad1f4d91195d301348ed (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit fcb2409598d37e2078076cf43794ef6c445ac22f
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Aug 29 21:59:16 2011 +0200
[1068] cleanup
commit da8bfe82aa18a67b1a99fa459f48cea89ee2a41a
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Aug 29 21:53:37 2011 +0200
[1068] missing initialization
commit 7980a6c8e598d34f5f733f5c6c3ca83c0a0f1187
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Aug 29 21:20:13 2011 +0200
[1068] one missing point: have the DatabaseUpdater's destructor catch
catch exception in rollback explicitly. added a test case for that.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/database.cc | 17 ++++++++++++++---
src/lib/datasrc/datasrc_messages.mes | 13 +++++++++++++
src/lib/datasrc/sqlite3_accessor.cc | 3 +--
src/lib/datasrc/tests/database_unittest.cc | 23 +++++++++++++++++++++--
4 files changed, 49 insertions(+), 7 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index c7564f4..db1d0fb 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -627,9 +627,20 @@ public:
virtual ~DatabaseUpdater() {
if (!committed_) {
- accessor_->rollbackUpdateZone();
- logger.info(DATASRC_DATABASE_UPDATER_ROLLBACK)
- .arg(zone_name_).arg(zone_class_).arg(db_name_);
+ try {
+ accessor_->rollbackUpdateZone();
+ logger.info(DATASRC_DATABASE_UPDATER_ROLLBACK)
+ .arg(zone_name_).arg(zone_class_).arg(db_name_);
+ } 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 rollbackUpdateZone() 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_UPDATER_ROLLBACKFAIL)
+ .arg(zone_name_).arg(zone_class_).arg(db_name_)
+ .arg(e.what());
+ }
}
logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_DESTROYED)
diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes
index 75bcc6a..efb88fd 100644
--- a/src/lib/datasrc/datasrc_messages.mes
+++ b/src/lib/datasrc/datasrc_messages.mes
@@ -608,6 +608,19 @@ the changes. The intermediate changes made through the updater won't
be applied to the underlying database. The zone name, its class, and
the underlying database name are shown in the log message.
+%DATASRC_DATABASE_UPDATER_ROLLBACKFAIL failed to roll back zone updates for '%1/%2' on %3: %4
+A zone updater is being destroyed without committing the changes to
+the database, and attempts to rollback incomplete updates, but it
+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. The zone name, its class, and the
+underlying database name as well as the error message thrown from the
+database module are shown in the log message.
+
% 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,
diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index 41da113..956f447 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -111,8 +111,7 @@ public:
}
void exec() {
- if (sqlite3_step(dbparameters_.statements_[stmt_id_]) !=
- SQLITE_DONE) {
+ if (sqlite3_step(dbparameters_.statements_[stmt_id_]) != SQLITE_DONE) {
sqlite3_reset(dbparameters_.statements_[stmt_id_]);
isc_throw(DataSourceError, "failed to " << desc_ << ": " <<
sqlite3_errmsg(dbparameters_.db_));
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 83a54f0..980b7f0 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -111,7 +111,7 @@ class MockAccessor : public NopAccessor {
Domains;
public:
- MockAccessor() {
+ MockAccessor() : rollbacked_(false) {
readonly_records_ = &readonly_records_master_;
update_records_ = &update_records_master_;
empty_records_ = &empty_records_master_;
@@ -328,6 +328,13 @@ public:
*readonly_records_ = *update_records_;
}
virtual void rollbackUpdateZone() {
+ // Special hook: if something with a name of "throw.example.org"
+ // has been added, trigger an imaginary unexpected event with an
+ // exception.
+ if (update_records_->count("throw.example.org.") > 0) {
+ isc_throw(DataSourceError, "unexpected failure in rollback");
+ }
+
rollbacked_ = true;
}
virtual void addRecordToZone(const string (&columns)[ADD_COLUMN_COUNT]) {
@@ -706,7 +713,6 @@ public:
}
// Helper methods for update tests
- //bool isRollbacked(bool expected = false) const {
bool isRollbacked() const {
return (update_accessor_->isRollbacked());
}
@@ -1572,6 +1578,19 @@ TEST_F(DatabaseClientTest, updateCancel) {
EXPECT_EQ(ZoneFinder::SUCCESS, finder->find(qname_, qtype_).code);
}
+TEST_F(DatabaseClientTest, exceptionFromRollback) {
+ updater_ = client_->getUpdater(zname_, true);
+
+ rrset_.reset(new RRset(Name("throw.example.org"), qclass_, qtype_,
+ rrttl_));
+ rrset_->addRdata(rdata::createRdata(rrset_->getType(),
+ rrset_->getClass(), "192.0.2.1"));
+ updater_->addRRset(*rrset_);
+ // destruct without commit. The added name will result in an exception
+ // in the MockAccessor's rollback method. It shouldn't be propagated.
+ EXPECT_NO_THROW(updater_.reset());
+}
+
TEST_F(DatabaseClientTest, duplicateCommit) {
// duplicate commit. should result in exception.
updater_ = client_->getUpdater(zname_, true);
More information about the bind10-changes
mailing list