BIND 10 trac1067, updated. 4db53f3593e24b80a33b608432ef463acbec295e [trac1067] Iterating through InMemory

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Aug 4 13:25:01 UTC 2011


The branch, trac1067 has been updated
       via  4db53f3593e24b80a33b608432ef463acbec295e (commit)
       via  009d45dfbb20a54ea402e7e8f18bc2d253f41ad6 (commit)
       via  f1d52ff7171da920acc7583fa427a95386312908 (commit)
      from  f33ffa77fdcc3e40ec42268ea09b67ac65982f1f (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 4db53f3593e24b80a33b608432ef463acbec295e
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Aug 4 15:24:34 2011 +0200

    [trac1067] Iterating through InMemory

commit 009d45dfbb20a54ea402e7e8f18bc2d253f41ad6
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Aug 4 12:15:24 2011 +0200

    [trac1067] Tests for in-memory iteration

commit f1d52ff7171da920acc7583fa427a95386312908
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Aug 4 12:14:15 2011 +0200

    [trac1067] The ZoneIterator interface

-----------------------------------------------------------------------

Summary of changes:
 src/lib/datasrc/Makefile.am                      |    2 +-
 src/lib/datasrc/client.h                         |    1 +
 src/lib/datasrc/iterator.h                       |   60 ++++++++++
 src/lib/datasrc/memory_datasrc.cc                |  133 ++++++++++++++++++----
 src/lib/datasrc/memory_datasrc.h                 |    4 +
 src/lib/datasrc/tests/memory_datasrc_unittest.cc |   39 +++++++
 6 files changed, 216 insertions(+), 23 deletions(-)
 create mode 100644 src/lib/datasrc/iterator.h

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/Makefile.am b/src/lib/datasrc/Makefile.am
index e6bff58..3f8f54d 100644
--- a/src/lib/datasrc/Makefile.am
+++ b/src/lib/datasrc/Makefile.am
@@ -21,7 +21,7 @@ libdatasrc_la_SOURCES += memory_datasrc.h memory_datasrc.cc
 libdatasrc_la_SOURCES += zone.h
 libdatasrc_la_SOURCES += result.h
 libdatasrc_la_SOURCES += logger.h logger.cc
-libdatasrc_la_SOURCES += client.h
+libdatasrc_la_SOURCES += client.h iterator.h
 libdatasrc_la_SOURCES += database.h database.cc
 libdatasrc_la_SOURCES += sqlite3_connection.h sqlite3_connection.cc
 nodist_libdatasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
diff --git a/src/lib/datasrc/client.h b/src/lib/datasrc/client.h
index 67e008f..6a3936a 100644
--- a/src/lib/datasrc/client.h
+++ b/src/lib/datasrc/client.h
@@ -25,6 +25,7 @@
 namespace isc {
 namespace datasrc {
 
+// The iterator.h is not included on purpose, most application won't need it
 class ZoneIterator;
 typedef boost::shared_ptr<ZoneIterator> ZoneIteratorPtr;
 
diff --git a/src/lib/datasrc/iterator.h b/src/lib/datasrc/iterator.h
new file mode 100644
index 0000000..3e3ce14
--- /dev/null
+++ b/src/lib/datasrc/iterator.h
@@ -0,0 +1,60 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/rrset.h>
+
+#include <boost/noncopyable.hpp>
+
+namespace isc {
+namespace datasrc {
+
+/**
+ * \brief Read-only iterator to a zone.
+ *
+ * You can get an instance of (descendand of) ZoneIterator from
+ * DataSourceClient::getIterator() method. The actual concrete implementation
+ * will be different depending on the actual data source used. This is the
+ * abstract interface.
+ *
+ * There's no way to start iterating from the beginning again or return.
+ */
+class ZoneIterator : public boost::noncopyable {
+public:
+    /**
+     * \brief Destructor
+     *
+     * Virtual destructor. It is empty, but ensures the right destructor from
+     * descendant is called.
+     */
+    virtual ~ ZoneIterator() { }
+    /**
+     * \brief Get next RRset from the zone.
+     *
+     * This returns the next RRset in the zone as a shared pointer. The
+     * shared pointer is used to allow both accessing in-memory data and
+     * automatic memory management.
+     *
+     * Any special order is not guaranteed.
+     *
+     * While this can potentially throw anything (including standard allocation
+     * errors), it should be rare.
+     *
+     * \return Pointer to the next RRset or NULL pointer when the iteration
+     *     gets to the end of the zone.
+     */
+    virtual isc::dns::ConstRRsetPtr getNextRRset() = 0;
+};
+
+}
+}
diff --git a/src/lib/datasrc/memory_datasrc.cc b/src/lib/datasrc/memory_datasrc.cc
index 26223da..5f7fce3 100644
--- a/src/lib/datasrc/memory_datasrc.cc
+++ b/src/lib/datasrc/memory_datasrc.cc
@@ -25,6 +25,8 @@
 #include <datasrc/memory_datasrc.h>
 #include <datasrc/rbtree.h>
 #include <datasrc/logger.h>
+#include <datasrc/iterator.h>
+#include <datasrc/data_source.h>
 
 using namespace std;
 using namespace isc::dns;
@@ -32,6 +34,27 @@ using namespace isc::dns;
 namespace isc {
 namespace datasrc {
 
+namespace {
+// Some type aliases
+/*
+ * Each domain consists of some RRsets. They will be looked up by the
+ * RRType.
+ *
+ * The use of map is questionable with regard to performance - there'll
+ * be usually only few RRsets in the domain, so the log n benefit isn't
+ * much and a vector/array might be faster due to its simplicity and
+ * continuous memory location. But this is unlikely to be a performance
+ * critical place and map has better interface for the lookups, so we use
+ * that.
+ */
+typedef map<RRType, ConstRRsetPtr> Domain;
+typedef Domain::value_type DomainPair;
+typedef boost::shared_ptr<Domain> DomainPtr;
+// The tree stores domains
+typedef RBTree<Domain> DomainTree;
+typedef RBNode<Domain> DomainNode;
+}
+
 // Private data and hidden methods of InMemoryZoneFinder
 struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
     // Constructor
@@ -44,25 +67,6 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         DomainPtr origin_domain(new Domain);
         origin_data_->setData(origin_domain);
     }
-
-    // Some type aliases
-    /*
-     * Each domain consists of some RRsets. They will be looked up by the
-     * RRType.
-     *
-     * The use of map is questionable with regard to performance - there'll
-     * be usually only few RRsets in the domain, so the log n benefit isn't
-     * much and a vector/array might be faster due to its simplicity and
-     * continuous memory location. But this is unlikely to be a performance
-     * critical place and map has better interface for the lookups, so we use
-     * that.
-     */
-    typedef map<RRType, ConstRRsetPtr> Domain;
-    typedef Domain::value_type DomainPair;
-    typedef boost::shared_ptr<Domain> DomainPtr;
-    // The tree stores domains
-    typedef RBTree<Domain> DomainTree;
-    typedef RBNode<Domain> DomainNode;
     static const DomainNode::Flags DOMAINFLAG_WILD = DomainNode::FLAG_USER1;
 
     // Information about the zone
@@ -634,7 +638,7 @@ InMemoryZoneFinder::load(const string& filename) {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_LOAD).arg(getOrigin()).
         arg(filename);
     // Load it into a temporary tree
-    InMemoryZoneFinderImpl::DomainTree tmp;
+    DomainTree tmp;
     masterLoad(filename.c_str(), getOrigin(), getClass(),
         boost::bind(&InMemoryZoneFinderImpl::addFromLoad, impl_, _1, &tmp));
     // If it went well, put it inside
@@ -700,8 +704,93 @@ InMemoryClient::addZone(ZoneFinderPtr zone_finder) {
 InMemoryClient::FindResult
 InMemoryClient::findZone(const isc::dns::Name& name) const {
     LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_FIND_ZONE).arg(name);
-    return (FindResult(impl_->zone_table.findZone(name).code,
-                       impl_->zone_table.findZone(name).zone));
+    ZoneTable::FindResult result(impl_->zone_table.findZone(name));
+    return (FindResult(result.code, result.zone));
 }
+
+namespace {
+
+class MemoryIterator : public ZoneIterator {
+private:
+    RBTreeNodeChain<Domain> chain_;
+    Domain::const_iterator dom_iterator_;
+    const DomainTree& tree_;
+    const DomainNode* node_;
+    bool ready_;
+public:
+    MemoryIterator(const DomainTree& tree, const Name& origin) :
+        tree_(tree),
+        ready_(true)
+    {
+        // Find the first node (origin) and preserve the node chain for future
+        // searches
+        DomainTree::Result result(tree_.find<void*>(origin, &node_, chain_,
+                                                    NULL, NULL));
+        // It can't happen that the origin is not in there
+        if (result != DomainTree::EXACTMATCH) {
+            isc_throw(Unexpected,
+                      "In-memory zone corrupted, missing origin node");
+        }
+        // Initialize the iterator if there's somewhere to point to
+        if (node_ != NULL && node_->getData() != DomainPtr()) {
+            dom_iterator_ = node_->getData()->begin();
+        }
+    }
+    virtual ConstRRsetPtr getNextRRset() {
+        if (!ready_) {
+            isc_throw(Unexpected, "Iterating past the zone end");
+        }
+        /*
+         * This cycle finds the first nonempty node with yet unused RRset.
+         * If it is NULL, we run out of nodes. If it is empty, it doesn't
+         * contain any RRsets. If we are at the end, just get to next one.
+         */
+        while (node_ != NULL && (node_->getData() == DomainPtr() ||
+                                 dom_iterator_ == node_->getData()->end())) {
+            node_ = tree_.nextNode(chain_);
+            // If there's a node, initialize the iterator and check next time
+            // if the map is empty or not
+            if (node_ != NULL && node_->getData() != NULL) {
+                dom_iterator_ = node_->getData()->begin();
+            }
+        }
+        if (node_ == NULL) {
+            // That's all, folks
+            ready_ = false;
+            return ConstRRsetPtr();
+        }
+        // The iterator points to the next yet unused RRset now
+        ConstRRsetPtr result(dom_iterator_->second);
+        // This one is used, move it to the next time for next call
+        ++ dom_iterator_;
+
+        return (result);
+    }
+};
+
+} // End of anonymous namespace
+
+ZoneIteratorPtr
+InMemoryClient::getIterator(const Name& name) const {
+    ZoneTable::FindResult result(impl_->zone_table.findZone(name));
+    if (result.code != result::SUCCESS) {
+        isc_throw(DataSourceError, "No such zone: " + name.toText());
+    }
+
+    const InMemoryZoneFinder*
+        zone(dynamic_cast<const InMemoryZoneFinder*>(result.zone.get()));
+    if (zone == NULL) {
+        /*
+         * TODO: This can happen only during some of the tests and only as
+         * a temporary solution. This should be fixed by #1159 and then
+         * this cast and check shouldn't be necessary. We don't have
+         * test for handling a "can not happen" condition.
+         */
+        isc_throw(Unexpected, "The zone at " + name.toText() +
+                  " is not InMemoryZoneFinder");
+    }
+    return ZoneIteratorPtr(new MemoryIterator(zone->impl_->domains_, name));
+}
+
 } // end of namespace datasrc
 } // end of namespace dns
diff --git a/src/lib/datasrc/memory_datasrc.h b/src/lib/datasrc/memory_datasrc.h
index 9707797..0f35f6e 100644
--- a/src/lib/datasrc/memory_datasrc.h
+++ b/src/lib/datasrc/memory_datasrc.h
@@ -182,6 +182,7 @@ private:
     struct InMemoryZoneFinderImpl;
     InMemoryZoneFinderImpl* impl_;
     //@}
+    friend class InMemoryClient;
 };
 
 /// \brief A data source client that holds all necessary data in memory.
@@ -258,6 +259,9 @@ public:
     /// For other details see \c DataSourceClient::findZone().
     virtual FindResult findZone(const isc::dns::Name& name) const;
 
+    /// \brief Implementation of the getIterator method
+    virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name) const;
+
 private:
     // TODO: Do we still need the PImpl if nobody should manipulate this class
     // directly any more (it should be handled through DataSourceClient)?
diff --git a/src/lib/datasrc/tests/memory_datasrc_unittest.cc b/src/lib/datasrc/tests/memory_datasrc_unittest.cc
index 22723fc..f47032f 100644
--- a/src/lib/datasrc/tests/memory_datasrc_unittest.cc
+++ b/src/lib/datasrc/tests/memory_datasrc_unittest.cc
@@ -29,6 +29,8 @@
 #include <dns/masterload.h>
 
 #include <datasrc/memory_datasrc.h>
+#include <datasrc/data_source.h>
+#include <datasrc/iterator.h>
 
 #include <gtest/gtest.h>
 
@@ -138,6 +140,43 @@ TEST_F(InMemoryClientTest, add_find_Zone) {
                   getOrigin());
 }
 
+TEST_F(InMemoryClientTest, iterator) {
+    // Just some preparations of data
+    boost::shared_ptr<InMemoryZoneFinder>
+        zone(new InMemoryZoneFinder(RRClass::IN(), Name("a")));
+    RRsetPtr aRRsetA(new RRset(Name("a"), RRClass::IN(), RRType::A(),
+                                  RRTTL(300)));
+    aRRsetA->addRdata(rdata::in::A("192.0.2.1"));
+    RRsetPtr aRRsetAAAA(new RRset(Name("a"), RRClass::IN(), RRType::AAAA(),
+                                  RRTTL(300)));
+    aRRsetAAAA->addRdata(rdata::in::AAAA("2001:db8::1"));
+    aRRsetAAAA->addRdata(rdata::in::AAAA("2001:db8::2"));
+    RRsetPtr subRRsetA(new RRset(Name("sub.x.a"), RRClass::IN(), RRType::A(),
+                                  RRTTL(300)));
+    subRRsetA->addRdata(rdata::in::A("192.0.2.2"));
+    EXPECT_EQ(result::SUCCESS, memory_client.addZone(zone));
+    // First, the zone is not there, so it should throw
+    EXPECT_THROW(memory_client.getIterator(Name("b")), DataSourceError);
+    // This zone is not there either, even when there's a zone containing this
+    EXPECT_THROW(memory_client.getIterator(Name("x.a")), DataSourceError);
+    // Now, an empty zone
+    ZoneIteratorPtr iterator(memory_client.getIterator(Name("a")));
+    EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
+    // It throws Unexpected when we are past the end
+    EXPECT_THROW(iterator->getNextRRset(), isc::Unexpected);
+    EXPECT_EQ(result::SUCCESS, zone->add(aRRsetA));
+    EXPECT_EQ(result::SUCCESS, zone->add(aRRsetAAAA));
+    EXPECT_EQ(result::SUCCESS, zone->add(subRRsetA));
+    // Check it with full zone, one by one.
+    // It should be in ascending order in case of InMemory data source
+    // (isn't guaranteed in general)
+    iterator = memory_client.getIterator(Name("a"));
+    EXPECT_EQ(aRRsetA, iterator->getNextRRset());
+    EXPECT_EQ(aRRsetAAAA, iterator->getNextRRset());
+    EXPECT_EQ(subRRsetA, iterator->getNextRRset());
+    EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
+}
+
 TEST_F(InMemoryClientTest, getZoneCount) {
     EXPECT_EQ(0, memory_client.getZoneCount());
     memory_client.addZone(




More information about the bind10-changes mailing list