BIND 10 trac1331, updated. 046729c74341bb2ed1e6f60f81470cf6a6883000 [1331] Tests for the journal
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Nov 9 08:10:57 UTC 2011
The branch, trac1331 has been updated
via 046729c74341bb2ed1e6f60f81470cf6a6883000 (commit)
via 36db2f897ac139ca9b71ccee07a7b1ba1e3aee7b (commit)
via 573abf93bec24753aebb5a6c70d8f50def521879 (commit)
via d287d9c92ecfb59d2c9f525cf79c7bb5167984f6 (commit)
from 8cea64b69af8d5ef21497d2f1c9812968ce5d8f7 (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 046729c74341bb2ed1e6f60f81470cf6a6883000
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Nov 8 18:50:31 2011 +0100
[1331] Tests for the journal
commit 36db2f897ac139ca9b71ccee07a7b1ba1e3aee7b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Nov 8 11:24:36 2011 +0100
[1331] A documentation refinement
commit 573abf93bec24753aebb5a6c70d8f50def521879
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Nov 8 10:59:18 2011 +0100
[1331] Propagate the journaling to constructor
commit d287d9c92ecfb59d2c9f525cf79c7bb5167984f6
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Nov 8 10:47:52 2011 +0100
[1331] Add journaling parameter to getUpdater
As the application can't call the constructor directly, it will be
passed from here.
Defaults to false as a backward compatibility (might be temporary)
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/client.h | 21 +++-
src/lib/datasrc/database.cc | 13 +-
src/lib/datasrc/database.h | 3 +-
src/lib/datasrc/memory_datasrc.cc | 2 +-
src/lib/datasrc/memory_datasrc.h | 3 +-
src/lib/datasrc/tests/client_unittest.cc | 4 +-
src/lib/datasrc/tests/database_unittest.cc | 244 ++++++++++++++++++++++++++++
src/lib/datasrc/zone.h | 12 ++
8 files changed, 293 insertions(+), 9 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/client.h b/src/lib/datasrc/client.h
index 40b7a3f..22a321f 100644
--- a/src/lib/datasrc/client.h
+++ b/src/lib/datasrc/client.h
@@ -265,6 +265,21 @@ public:
/// In such cases this method will result in an \c isc::NotImplemented
/// exception unconditionally or when \c replace is false).
///
+ /// If \c journaling is true, the data source should store a journal
+ /// of changes. These can be used later on by, for example, IXFR-out.
+ /// However, the parameter is a hint only. It might be unable to store
+ /// them and they would be silently discarded. Or it might need to
+ /// store them no matter what (for example a git-based data source would
+ /// store journal implicitly). When the \c journaling is true, it might
+ /// require that the following update be formated as IXFR transfer
+ /// (SOA to be removed, bunch of RRs to be removed, SOA to be added,
+ /// bunch of RRs to be added, and possibly repeated). If it is false, it
+ /// must not require so.
+ ///
+ /// We don't support erasing the whole zone (by replace being true) and
+ /// saving a journal at the same time. In such situation, BadValue is
+ /// thrown.
+ ///
/// \note To avoid throwing the exception accidentally with a lazy
/// implementation, we still keep this method pure virtual without
/// an implementation. All derived classes must explicitly define this
@@ -275,14 +290,18 @@ public:
/// \exception DataSourceError Internal error in the underlying data
/// source.
/// \exception std::bad_alloc Resource allocation failure.
+ /// \exception BadValue if both replace and journaling are true.
///
/// \param name The zone name to be updated
/// \param replace Whether to delete existing RRs before making updates
+ /// \param journaling The zone updater should store a journal of the
+ /// changes.
///
/// \return A pointer to the updater; it will be NULL if the specified
/// zone isn't found.
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
- bool replace) const = 0;
+ bool replace, bool journaling = false)
+ const = 0;
};
}
}
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 3b079c6..9ee344f 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -827,10 +827,11 @@ DatabaseClient::getIterator(const isc::dns::Name& name) const {
class DatabaseUpdater : public ZoneUpdater {
public:
DatabaseUpdater(shared_ptr<DatabaseAccessor> accessor, int zone_id,
- const Name& zone_name, const RRClass& zone_class) :
+ const Name& zone_name, const RRClass& zone_class,
+ bool journaling) :
committed_(false), accessor_(accessor), zone_id_(zone_id),
db_name_(accessor->getDBName()), zone_name_(zone_name.toText()),
- zone_class_(zone_class),
+ zone_class_(zone_class), journaling_(journaling),
finder_(new DatabaseClient::Finder(accessor_, zone_id_, zone_name))
{
logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_CREATED)
@@ -872,6 +873,7 @@ private:
const string db_name_;
const string zone_name_;
const RRClass zone_class_;
+ const bool journaling_;
boost::scoped_ptr<DatabaseClient::Finder> finder_;
};
@@ -975,7 +977,10 @@ DatabaseUpdater::commit() {
// The updater factory
ZoneUpdaterPtr
-DatabaseClient::getUpdater(const isc::dns::Name& name, bool replace) const {
+DatabaseClient::getUpdater(const isc::dns::Name& name, bool replace,
+ bool journaling) const
+{
+ // TODO: Handle journaling (pass it to the updater)
shared_ptr<DatabaseAccessor> update_accessor(accessor_->clone());
const std::pair<bool, int> zone(update_accessor->startUpdateZone(
name.toText(), replace));
@@ -984,7 +989,7 @@ DatabaseClient::getUpdater(const isc::dns::Name& name, bool replace) const {
}
return (ZoneUpdaterPtr(new DatabaseUpdater(update_accessor, zone.second,
- name, rrclass_)));
+ name, rrclass_, journaling)));
}
}
}
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index b9379b7..a0c1e0d 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -872,7 +872,8 @@ public:
/// accessor. The returned updater will be able to work separately from
/// the original client.
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
- bool replace) const;
+ bool replace,
+ bool journaling = false) const;
private:
/// \brief The RR class that this client handles.
diff --git a/src/lib/datasrc/memory_datasrc.cc b/src/lib/datasrc/memory_datasrc.cc
index 8da43d0..4ac8e2a 100644
--- a/src/lib/datasrc/memory_datasrc.cc
+++ b/src/lib/datasrc/memory_datasrc.cc
@@ -811,7 +811,7 @@ InMemoryClient::getIterator(const Name& name) const {
}
ZoneUpdaterPtr
-InMemoryClient::getUpdater(const isc::dns::Name&, bool) const {
+InMemoryClient::getUpdater(const isc::dns::Name&, bool, bool) const {
isc_throw(isc::NotImplemented, "Update attempt on in memory data source");
}
diff --git a/src/lib/datasrc/memory_datasrc.h b/src/lib/datasrc/memory_datasrc.h
index 610deff..2fa70ee 100644
--- a/src/lib/datasrc/memory_datasrc.h
+++ b/src/lib/datasrc/memory_datasrc.h
@@ -283,7 +283,8 @@ public:
/// to update via its updater (this may or may not be a good idea and
/// is subject to further discussions).
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
- bool replace) const;
+ bool replace, bool journaling = false)
+ const;
private:
// TODO: Do we still need the PImpl if nobody should manipulate this class
diff --git a/src/lib/datasrc/tests/client_unittest.cc b/src/lib/datasrc/tests/client_unittest.cc
index 5b2c91a..ade6fc7 100644
--- a/src/lib/datasrc/tests/client_unittest.cc
+++ b/src/lib/datasrc/tests/client_unittest.cc
@@ -32,7 +32,9 @@ public:
virtual FindResult findZone(const isc::dns::Name&) const {
return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
}
- virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name&, bool) const {
+ virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name&, bool, bool)
+ const
+ {
return (ZoneUpdaterPtr());
}
};
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 1514fc3..0748cc0 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -13,6 +13,7 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <boost/shared_ptr.hpp>
+#include <boost/lexical_cast.hpp>
#include <gtest/gtest.h>
@@ -37,6 +38,7 @@ using namespace std;
// for some systems.
using boost::shared_ptr;
using boost::dynamic_pointer_cast;
+using boost::lexical_cast;
using namespace isc::dns;
namespace {
@@ -264,6 +266,40 @@ private:
};
+/**
+ * Single journal entry in the mock database.
+ *
+ * All the members there are public for simplicity, as it only stores data.
+ * We use the implicit constructor and operator. The members can't be const
+ * because of the assignment operator (used in the vectors).
+ */
+struct JournalEntry {
+ JournalEntry(int id, uint32_t serial,
+ DatabaseAccessor::DiffOperation operation,
+ const std::string (&data)[DatabaseAccessor::DIFF_PARAM_COUNT])
+ : id_(id), serial_(serial), operation_(operation), data_(data)
+ {}
+ JournalEntry(int id, uint32_t serial,
+ DatabaseAccessor::DiffOperation operation,
+ const std::string& name, const std::string& type,
+ const std::string& ttl, const std::string& rdata):
+ id_(id), serial_(serial), operation_(operation)
+ {
+ data_[DatabaseAccessor::DIFF_NAME] = name;
+ data_[DatabaseAccessor::DIFF_TYPE] = type;
+ data_[DatabaseAccessor::DIFF_TTL] = ttl;
+ data_[DatabaseAccessor::DIFF_RDATA] = rdata;
+ }
+ int id_;
+ uint32_t serial_;
+ DatabaseAccessor::DiffOperation operation_;
+ std::string data_[DatabaseAccessor::DIFF_PARAM_COUNT];
+ bool operator==(const JournalEntry& other) const {
+ return (id_ == other.id_ && serial_ == other.serial_ &&
+ operation_ == other.operation_ && data_ == other.data_);
+ }
+};
+
/*
* A virtual database accessor that pretends it contains single zone --
* example.org.
@@ -646,6 +682,30 @@ public:
isc_throw(isc::Unexpected, "Unknown zone ID");
}
}
+ virtual void addRecordDiff(int id, uint32_t serial,
+ DiffOperation operation,
+ const std::string (&data)[DIFF_PARAM_COUNT])
+ {
+ if (id == 13) { // The null zone doesn't support journaling
+ isc_throw(isc::NotImplemented, "Test not implemented behaviour");
+ } else if (id == -1) { // Bad zone throws
+ isc_throw(DataSourceError, "Test error");
+ } else {
+ journal_entries_.push_back(JournalEntry(id, serial, operation,
+ data));
+ }
+ }
+
+ // Check the journal is as expected and clear the journal
+ void checkJournal(const std::vector<JournalEntry> &expected) {
+ std::vector<JournalEntry> journal;
+ // Clean the journal, but keep local copy to check
+ journal.swap(journal_entries_);
+ ASSERT_EQ(expected.size(), journal.size());
+ for (size_t i(0); i < expected.size(); ++ i) {
+ EXPECT_TRUE(expected[i] == journal[i]);
+ }
+ }
private:
// The following member variables are storage and/or update work space
@@ -665,6 +725,9 @@ private:
const Domains empty_records_master_;
const Domains* empty_records_;
+ // The journal data
+ std::vector<JournalEntry> journal_entries_;
+
// used as temporary storage after searchForRecord() and during
// getNextRecord() calls, as well as during the building of the
// fake data
@@ -782,6 +845,10 @@ public:
rrset_.reset(new RRset(qname_, qclass_, qtype_, rrttl_));
rrset_->addRdata(rdata::createRdata(rrset_->getType(),
rrset_->getClass(), "192.0.2.2"));
+ soa_.reset(new RRset(zname_, qclass_, RRType::SOA(), rrttl_));
+ soa_->addRdata(rdata::createRdata(soa_->getType(), soa_->getClass(),
+ "ns1.example.org. admin.example.org. "
+ "1234 3600 1800 2419200 7200"));
// And its RRSIG. Also different from the configured one.
rrsigset_.reset(new RRset(qname_, qclass_, RRType::RRSIG(),
@@ -883,6 +950,7 @@ public:
const RRTTL rrttl_; // commonly used RR TTL
RRsetPtr rrset_; // for adding/deleting an RRset
RRsetPtr rrsigset_; // for adding/deleting an RRset
+ RRsetPtr soa_; // for adding/deleting an RRset
// update related objects to be tested
ZoneUpdaterPtr updater_;
@@ -2627,4 +2695,180 @@ TEST_F(MockDatabaseClientTest, badName) {
DataSourceError);
}
+/*
+ * Test correct use of the updater with a journal.
+ */
+TEST_F(MockDatabaseClientTest, journal) {
+ updater_ = client_->getUpdater(zname_, false, true);
+ updater_->deleteRRset(*soa_);
+ updater_->deleteRRset(*rrset_);
+ soa_.reset(new RRset(zname_, qclass_, RRType::SOA(), rrttl_));
+ soa_->addRdata(rdata::createRdata(soa_->getType(), soa_->getClass(),
+ "ns1.example.org. admin.example.org. "
+ "1234 3600 1800 2419201 7200"));
+ updater_->addRRset(*soa_);
+ updater_->addRRset(*rrset_);
+ ASSERT_NO_THROW(updater_->commit());
+ std::vector<JournalEntry> expected;
+ // For some reason, the example.org is called read only zone here.
+ expected.push_back(JournalEntry(READONLY_ZONE_ID, 2419200,
+ DatabaseAccessor::DIFF_DELETE,
+ "example.org.", "SOA", "3600",
+ "ns1.example.org. admin.example.org. "
+ "1234 3600 1800 2419200 7200"));
+ expected.push_back(JournalEntry(READONLY_ZONE_ID, 2419200,
+ DatabaseAccessor::DIFF_DELETE,
+ "example.org.", "A", "3600",
+ "192.0.2.2"));
+ expected.push_back(JournalEntry(READONLY_ZONE_ID, 2419201,
+ DatabaseAccessor::DIFF_ADD,
+ "example.org.", "SOA", "3600",
+ "ns1.example.org. admin.example.org. "
+ "1234 3600 1800 2419201 7200"));
+ expected.push_back(JournalEntry(READONLY_ZONE_ID, 2419200,
+ DatabaseAccessor::DIFF_ADD,
+ "example.org.", "A", "3600",
+ "192.0.2.2"));
+ current_accessor_->checkJournal(expected);
+}
+
+/*
+ * Push multiple delete-add sequences. Checks it is allowed and all is
+ * saved.
+ */
+TEST_F(MockDatabaseClientTest, journalMultiple) {
+ std::vector<JournalEntry> expected;
+ updater_ = client_->getUpdater(zname_, false, true);
+ std::string soa_rdata = "ns1.example.org. admin.example.org. "
+ "1234 3600 1800 2419200 7200";
+ for (size_t i(1); i < 100; ++ i) {
+ // Remove the old SOA
+ updater_->deleteRRset(*soa_);
+ expected.push_back(JournalEntry(READONLY_ZONE_ID, 2419200 + i - 1,
+ DatabaseAccessor::DIFF_DELETE,
+ "example.org.", "SOA", "3600",
+ soa_rdata));
+ // Create a new SOA
+ soa_rdata = "ns1.example.org. admin.example.org. 1234 3600 1800 " +
+ lexical_cast<std::string>(2419200 + i) + " 7200";
+ soa_.reset(new RRset(zname_, qclass_, RRType::SOA(), rrttl_));
+ soa_->addRdata(rdata::createRdata(soa_->getType(), soa_->getClass(),
+ soa_rdata));
+ // Add the new SOA
+ updater_->addRRset(*soa_);
+ expected.push_back(JournalEntry(READONLY_ZONE_ID, 2419200 + i,
+ DatabaseAccessor::DIFF_DELETE,
+ "example.org.", "SOA", "3600",
+ soa_rdata));
+ }
+ ASSERT_NO_THROW(updater_->commit());
+ // Check the journal contains everything.
+ current_accessor_->checkJournal(expected);
+}
+
+/*
+ * Test passing a forbidden sequence to it and expect it to throw.
+ *
+ * Note that we implicitly test in different testcases (these for add and
+ * delete) that if the journaling is false, it doesn't expect the order.
+ */
+TEST_F(MockDatabaseClientTest, journalBadSequence) {
+ std::vector<JournalEntry> expected;
+ {
+ SCOPED_TRACE("Delete A before SOA");
+ updater_ = client_->getUpdater(zname_, false, true);
+ EXPECT_THROW(updater_->deleteRRset(*rrset_), isc::BadValue);
+ // Make sure the journal is empty now
+ current_accessor_->checkJournal(expected);
+ }
+
+ {
+ SCOPED_TRACE("Add before delete");
+ updater_ = client_->getUpdater(zname_, false, true);
+ EXPECT_THROW(updater_->addRRset(*soa_), isc::BadValue);
+ // Make sure the journal is empty now
+ current_accessor_->checkJournal(expected);
+ }
+
+ {
+ SCOPED_TRACE("Add A before SOA");
+ updater_ = client_->getUpdater(zname_, false, true);
+ // So far OK
+ EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
+ // But we miss the add SOA here
+ EXPECT_THROW(updater_->addRRset(*rrset_), isc::BadValue);
+ // Make sure the journal contains only the first one
+ expected.push_back(JournalEntry(READONLY_ZONE_ID, 2419200,
+ DatabaseAccessor::DIFF_DELETE,
+ "example.org.", "SOA", "3600",
+ "ns1.example.org. admin.example.org. "
+ "1234 3600 1800 2419200 7200"));
+ current_accessor_->checkJournal(expected);
+ }
+
+ {
+ SCOPED_TRACE("Commit before add");
+ updater_ = client_->getUpdater(zname_, false, true);
+ // So far OK
+ EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
+ // Commit at the wrong time
+ EXPECT_THROW(updater_->commit(), isc::BadValue);
+ current_accessor_->checkJournal(expected);
+ }
+
+ {
+ SCOPED_TRACE("Delete two SOAs");
+ // So far OK
+ EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
+ // Delete the SOA again
+ EXPECT_THROW(updater_->addRRset(*soa_), isc::BadValue);
+ current_accessor_->checkJournal(expected);
+ }
+
+ {
+ SCOPED_TRACE("Add two SOAs");
+ // So far OK
+ EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
+ // Still OK
+ EXPECT_NO_THROW(updater_->addRRset(*soa_));
+ // But this one is added again
+ EXPECT_THROW(updater_->addRRset(*soa_), isc::BadValue);
+ expected.push_back(JournalEntry(READONLY_ZONE_ID, 2419200,
+ DatabaseAccessor::DIFF_ADD,
+ "example.org.", "SOA", "3600",
+ "ns1.example.org. admin.example.org. "
+ "1234 3600 1800 2419200 7200"));
+ current_accessor_->checkJournal(expected);
+ }
+}
+
+/*
+ * Test it rejects to store journals when we request it together with
+ * erasing the whole zone.
+ */
+TEST_F(MockDatabaseClientTest, journalOnErase) {
+ EXPECT_THROW(client_->getUpdater(zname_, true, true), isc::BadValue);
+}
+
+/*
+ * Check that the exception isn't raised when the journal is not implemented.
+ */
+TEST_F(MockDatabaseClientTest, journalNotImplemented) {
+ updater_ = client_->getUpdater(Name("null.example.org"), false, true);
+ EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
+ soa_.reset(new RRset(zname_, qclass_, RRType::SOA(), rrttl_));
+ soa_->addRdata(rdata::createRdata(soa_->getType(), soa_->getClass(),
+ "ns1.example.org. admin.example.org. "
+ "1234 3600 1800 2419201 7200"));
+ EXPECT_NO_THROW(updater_->addRRset(*soa_));
+}
+
+/*
+ * Test that different exceptions are propagated.
+ */
+TEST_F(MockDatabaseClientTest, journalException) {
+ updater_ = client_->getUpdater(Name("bad.example.org"), false, true);
+ EXPECT_THROW(updater_->deleteRRset(*soa_), DataSourceError);
+}
+
}
diff --git a/src/lib/datasrc/zone.h b/src/lib/datasrc/zone.h
index fa1c744..97ab03d 100644
--- a/src/lib/datasrc/zone.h
+++ b/src/lib/datasrc/zone.h
@@ -438,6 +438,10 @@ public:
/// calls after \c commit() the implementation must throw a
/// \c DataSourceError exception.
///
+ /// If journaling was requested when getting this updater, it might reject
+ /// to add the RRset if the squence doesn't look like and IXFR. In such
+ /// such case isc::BadValue is thrown.
+ ///
/// \todo As noted above we may have to revisit the design details as we
/// gain experiences:
///
@@ -454,6 +458,8 @@ public:
///
/// \exception DataSourceError Called after \c commit(), RRset is invalid
/// (see above), internal data source error
+ /// \exception isc::BadValue Journaling is enabled and the current RRset
+ /// doesn't fit into the IXFR sequence (see above).
/// \exception std::bad_alloc Resource allocation failure
///
/// \param rrset The RRset to be added
@@ -503,6 +509,10 @@ public:
/// calls after \c commit() the implementation must throw a
/// \c DataSourceError exception.
///
+ /// If journaling was requested when getting this updater, it might reject
+ /// to add the RRset if the squence doesn't look like and IXFR. In such
+ /// such case isc::BadValue is thrown.
+ ///
/// \todo As noted above we may have to revisit the design details as we
/// gain experiences:
///
@@ -520,6 +530,8 @@ public:
///
/// \exception DataSourceError Called after \c commit(), RRset is invalid
/// (see above), internal data source error
+ /// \exception isc::BadValue Journaling is enabled and the current RRset
+ /// doesn't fit into the IXFR sequence (see above).
/// \exception std::bad_alloc Resource allocation failure
///
/// \param rrset The RRset to be deleted
More information about the bind10-changes
mailing list