BIND 10 trac3360, updated. 26ddda6791c9d0026c16359ca6906865316c35ca [3360] Cleanup in the Memfile backend code.

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Mar 25 10:54:06 UTC 2014


The branch, trac3360 has been updated
       via  26ddda6791c9d0026c16359ca6906865316c35ca (commit)
      from  56d75260f0a2a5c99117910cde00e18ea330183b (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 26ddda6791c9d0026c16359ca6906865316c35ca
Author: Marcin Siodelski <marcin at isc.org>
Date:   Tue Mar 25 11:25:24 2014 +0100

    [3360] Cleanup in the Memfile backend code.

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

Summary of changes:
 doc/guide/bind10-guide.xml                         |   20 +------
 src/lib/dhcpsrv/dhcpsrv_messages.mes               |   25 +++++++++
 src/lib/dhcpsrv/memfile_lease_mgr.cc               |   59 +++++++++++++++-----
 src/lib/dhcpsrv/memfile_lease_mgr.h                |    9 +--
 src/lib/dhcpsrv/tests/alloc_engine_unittest.cc     |    4 +-
 src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc  |    6 +-
 .../dhcpsrv/tests/memfile_lease_mgr_unittest.cc    |   28 +++++++++-
 7 files changed, 107 insertions(+), 44 deletions(-)

-----------------------------------------------------------------------
diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml
index f666411..89fc6fa 100644
--- a/doc/guide/bind10-guide.xml
+++ b/doc/guide/bind10-guide.xml
@@ -3714,15 +3714,7 @@ Dhcp4/subnet4	[]	list	(default)
       <title>Database Configuration</title>
       <para>
       All leases issued by the server are stored in the lease database.  Currently,
-      the only supported database is MySQL
-      <footnote>
-      <para>
-      The server comes with an in-memory database ("memfile") configured as the default
-      database. This is used for internal testing and is not supported.  In addition,
-      it does not store lease information on disk: lease information will be lost if the
-      server is restarted.
-      </para>
-      </footnote>, and so the server must be configured to
+      the only supported database is MySQL and so the server must be configured to
       access the correct database with the appropriate credentials.
       </para>
         <note>
@@ -4879,15 +4871,7 @@ Dhcp6/subnet6/	list
       <title>Database Configuration</title>
       <para>
       All leases issued by the server are stored in the lease database.  Currently,
-      the only supported database is MySQL
-      <footnote>
-      <para>
-      The server comes with an in-memory database ("memfile") configured as the default
-      database. This is used for internal testing and is not supported.  In addition,
-      it does not store lease information on disk: lease information will be lost if the
-      server is restarted.
-      </para>
-      </footnote>, and so the server must be configured to
+      the only supported database is MySQL, and so the server must be configured to
       access the correct database with the appropriate credentials.
       </para>
       <note>
diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes
index b5ff188..df8fa99 100644
--- a/src/lib/dhcpsrv/dhcpsrv_messages.mes
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes
@@ -279,6 +279,31 @@ subnet ID and hardware address.
 A debug message issued when the server is about to obtain schema version
 information from the memory file database.
 
+% DHCPSRV_MEMFILE_LEASE_LOAD4 loading lease %1
+A debug message issued when DHCPv4 lease is being loaded from the file to
+memory.
+
+% DHCPSRV_MEMFILE_LEASE_LOAD6 loading lease %1
+A debug message issued when DHCPv6 lease is being loaded from the file to
+memory.
+
+% DHCPSRV_MEMFILE_LEASES_RELOAD4 reloading leases from %1
+An info message issued when server is about to start reading DHCPv4 leases
+from the lease file. All leases currently held in the memory will be
+replaced by those read from the file.
+
+% DHCPSRV_MEMFILE_LEASES_RELOAD6 reloading leases from %1
+An info message issued when server is about to start reading DHCPv6 leases
+from the lease file. All leases currently held in the memory will be
+replaced by those read from the file.
+
+% DHCPSRV_MEMFILE_NO_STORAGE running in non-persistent mode, leases will be lost after restart
+A warning message issued when writes of leases to disk have been disabled
+in the configuration. This mode is useful for some kinds of performance
+testing but should not be enabled in normal circumstances. Non-persistence
+mode is enabled when 'persist4=no persist6=no' parameters are specified
+in the database access string.
+
 % DHCPSRV_MEMFILE_ROLLBACK rolling back memory file database
 The code has issued a rollback call.  For the memory file database, this is
 a no-op.
diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc
index 03fa124..64cce0e 100644
--- a/src/lib/dhcpsrv/memfile_lease_mgr.cc
+++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc
@@ -36,6 +36,14 @@ Memfile_LeaseMgr::Memfile_LeaseMgr(const ParameterMap& parameters)
         lease_file6_->open();
         load6();
     }
+
+    // If lease persistence have been disabled for both v4 and v6,
+    // issue a warning. It is ok not to write leases to disk when
+    // doing testing, but it should not be done in normal server
+    // operation.
+    if (!persistLeases(V4) && !persistLeases(V6)) {
+        LOG_WARN(dhcpsrv_logger, DHCPSRV_MEMFILE_NO_STORAGE);
+    }
 }
 
 Memfile_LeaseMgr::~Memfile_LeaseMgr() {
@@ -62,7 +70,7 @@ Memfile_LeaseMgr::addLease(const Lease4Ptr& lease) {
     // Try to write a lease to disk first. If this fails, the lease will
     // not be inserted to the memory and the disk and in-memory data will
     // remain consistent.
-    if (lease_file4_) {
+    if (persistLeases(V4)) {
         lease_file4_->append(*lease);
     }
 
@@ -83,7 +91,7 @@ Memfile_LeaseMgr::addLease(const Lease6Ptr& lease) {
     // Try to write a lease to disk first. If this fails, the lease will
     // not be inserted to the memory and the disk and in-memory data will
     // remain consistent.
-    if (lease_file6_) {
+    if (persistLeases(V6)) {
         lease_file6_->append(*lease);
     }
 
@@ -290,7 +298,7 @@ Memfile_LeaseMgr::updateLease4(const Lease4Ptr& lease) {
     // Try to write a lease to disk first. If this fails, the lease will
     // not be inserted to the memory and the disk and in-memory data will
     // remain consistent.
-    if (lease_file4_) {
+    if (persistLeases(V4)) {
         lease_file4_->append(*lease);
     }
 
@@ -311,7 +319,7 @@ Memfile_LeaseMgr::updateLease6(const Lease6Ptr& lease) {
     // Try to write a lease to disk first. If this fails, the lease will
     // not be inserted to the memory and the disk and in-memory data will
     // remain consistent.
-    if (lease_file6_) {
+    if (persistLeases(V6)) {
         lease_file6_->append(*lease);
     }
 
@@ -329,7 +337,7 @@ Memfile_LeaseMgr::deleteLease(const isc::asiolink::IOAddress& addr) {
             // No such lease
             return (false);
         } else {
-            if (lease_file4_) {
+            if (persistLeases(V4)) {
                 // Copy the lease. The valid lifetime needs to be modified and
                 // we don't modify the original lease.
                 Lease4 lease_copy = **l;
@@ -349,7 +357,7 @@ Memfile_LeaseMgr::deleteLease(const isc::asiolink::IOAddress& addr) {
             // No such lease
             return (false);
         } else {
-            if (lease_file6_) {
+            if (persistLeases(V6)) {
                 // Copy the lease. The lifetimes need to be modified and we
                 // don't modify the original lease.
                 Lease6 lease_copy = **l;
@@ -415,17 +423,23 @@ Memfile_LeaseMgr::persistLeases(Universe u) const {
 
 std::string
 Memfile_LeaseMgr::initLeaseFilePath(Universe u) {
-    bool persist = true;
+    std::string persist_val;
+    std::string persist_str = (u == V4 ? "persist4" : "persist6");
     try {
-        std::string persist_str = getParameter("persist");
-        if (persist_str == "no") {
-            persist = false;
-        }
+        persist_val = getParameter(persist_str);
     } catch (const Exception& ex) {
-        persist = true;
+        // If parameter persist hasn't been specified, we use a default value
+        // 'yes'.
+        persist_val = "yes";
     }
-    if (!persist) {
+    // If persist_val is 'no' we will not store leases to disk, so let's
+    // return empty file name.
+    if (persist_val == "no") {
         return ("");
+
+    } else if (persist_val != "yes") {
+        isc_throw(isc::BadValue, "invalid value '" << persist_str
+                  << "=" << persist_val << "'");
     }
 
     std::string param_name = (u == V4 ? "leasefile4" : "leasefile6");
@@ -442,9 +456,13 @@ void
 Memfile_LeaseMgr::load4() {
     // If lease file hasn't been opened, we are working in non-persistent mode.
     // That's fine, just leave.
-    if (!lease_file4_) {
+    if (!persistLeases(V4)) {
         return;
     }
+
+    LOG_INFO(dhcpsrv_logger, DHCPSRV_MEMFILE_LEASES_RELOAD4)
+        .arg(lease_file4_->getFilename());
+
     // Remove existing leases (if any). We will recreate them based on the
     // data on disk.
     storage4_.clear();
@@ -462,6 +480,9 @@ Memfile_LeaseMgr::load4() {
         // If we got the lease, we update the internal container holding
         // leases. Otherwise, we reached the end of file and we leave.
         if (lease) {
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL_DATA,
+                      DHCPSRV_MEMFILE_LEASE_LOAD4)
+                .arg(lease->toText());
             loadLease4(lease);
         }
     } while (lease);
@@ -496,9 +517,13 @@ void
 Memfile_LeaseMgr::load6() {
     // If lease file hasn't been opened, we are working in non-persistent mode.
     // That's fine, just leave.
-    if (!lease_file6_) {
+    if (!persistLeases(V6)) {
         return;
     }
+
+    LOG_INFO(dhcpsrv_logger, DHCPSRV_MEMFILE_LEASES_RELOAD6)
+        .arg(lease_file6_->getFilename());
+
     // Remove existing leases (if any). We will recreate them based on the
     // data on disk.
     storage6_.clear();
@@ -516,6 +541,10 @@ Memfile_LeaseMgr::load6() {
         // If we got the lease, we update the internal container holding
         // leases. Otherwise, we reached the end of file and we leave.
         if (lease) {
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL_DATA,
+                      DHCPSRV_MEMFILE_LEASE_LOAD6)
+                .arg(lease->toText());
+
             loadLease6(lease);
         }
     } while (lease);
diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h
index 74a6ad6..58b3131 100644
--- a/src/lib/dhcpsrv/memfile_lease_mgr.h
+++ b/src/lib/dhcpsrv/memfile_lease_mgr.h
@@ -57,10 +57,11 @@ namespace dhcp {
 ///
 /// Originally, the Memfile backend didn't write leases to disk. This was
 /// particularly useful for testing server performance in non-disk bound
-/// conditions. In order to preserve this capability, the new parameter
-/// "persistence=yes/no" in the data base access string has been introduced.
-/// For example, database access string: "type=memfile persistence=no"
-/// disables disk writes to disk.
+/// conditions. In order to preserve this capability, the new parameters
+/// "persist4=yes|no" and "persist6=yes|no" has been introduced in the
+/// database access string. For example, database access string:
+/// "type=memfile persist4=no persist6=yes" disables disk writes to disk
+/// of DHCPv4 leases enables writes to disk of DHCPv6 leases.
 ///
 /// The lease file locations can be specified for DHCPv4 and DHCPv6 leases
 /// with the following two parameters in the database access string:
diff --git a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
index 47cbec3..4930d32 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
@@ -101,7 +101,7 @@ public:
 
         initFqdn("", false, false);
 
-        factory_.create("type=memfile persist=no");
+        factory_.create("type=memfile persist4=no persist6=no");
     }
 
     /// @brief Configures a subnet and adds one pool to it.
@@ -424,7 +424,7 @@ public:
         subnet_->addPool(pool_);
         cfg_mgr.addSubnet4(subnet_);
 
-        factory_.create("type=memfile persist=no");
+        factory_.create("type=memfile persist4=no persist6=no");
     }
 
     /// @brief checks if Lease4 matches expected configuration
diff --git a/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc b/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc
index 7d1620b..9dff824 100644
--- a/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc
+++ b/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc
@@ -410,7 +410,8 @@ TEST_F(DbAccessParserTest, commit) {
             }, isc::dhcp::NoLeaseManager);
 
     // Set up the parser to open the memfile database.
-    const char* config[] = {"type", "memfile", "persist", "no", NULL};
+    const char* config[] = {"type", "memfile", "persist4", "no",
+                            "persist6", "no", NULL};
     string json_config = toJson(config);
 
     ConstElementPtr json_elements = Element::fromJSON(json_config);
@@ -420,7 +421,8 @@ TEST_F(DbAccessParserTest, commit) {
     EXPECT_NO_THROW(parser.build(json_elements));
 
     // Ensure that the access string is as expected.
-    EXPECT_EQ(std::string("persist=no type=memfile"), parser.getDbAccessString());
+    EXPECT_EQ(std::string("persist4=no persist6=no type=memfile"),
+              parser.getDbAccessString());
 
     // Committal of the parser changes should open the database.
     EXPECT_NO_THROW(parser.commit());
diff --git a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc
index 9e1c02c..dfaaf29 100644
--- a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc
@@ -113,10 +113,31 @@ public:
 TEST_F(MemfileLeaseMgrTest, constructor) {
 
     LeaseMgr::ParameterMap pmap;
-    pmap["persist"] = "no";
+    pmap["persist4"] = "no";
+    pmap["persist6"] = "no";
     boost::scoped_ptr<Memfile_LeaseMgr> lease_mgr;
 
-    ASSERT_NO_THROW(lease_mgr.reset(new Memfile_LeaseMgr(pmap)));
+    EXPECT_NO_THROW(lease_mgr.reset(new Memfile_LeaseMgr(pmap)));
+
+    pmap["persist4"] = "yes";
+    pmap["persist6"] = "yes";
+    pmap["leasefile4"] = getLeaseFilePath("leasefile4_1.csv");
+    pmap["leasefile6"] = getLeaseFilePath("leasefile6_1.csv");
+    EXPECT_NO_THROW(lease_mgr.reset(new Memfile_LeaseMgr(pmap)));
+
+    // Expecting that persist parameter is yes or no. Everything other than
+    // that is wrong.
+    pmap["persist4"] = "bogus";
+    pmap["persist6"] = "yes";
+    pmap["leasefile4"] = getLeaseFilePath("leasefile4_1.csv");
+    pmap["leasefile6"] = getLeaseFilePath("leasefile6_1.csv");
+    EXPECT_THROW(lease_mgr.reset(new Memfile_LeaseMgr(pmap)), isc::BadValue);
+
+    pmap["persist4"] = "yes";
+    pmap["persist6"] = "bogus";
+    pmap["leasefile4"] = getLeaseFilePath("leasefile4_1.csv");
+    pmap["leasefile6"] = getLeaseFilePath("leasefile6_1.csv");
+    EXPECT_THROW(lease_mgr.reset(new Memfile_LeaseMgr(pmap)), isc::BadValue);
 }
 
 // Checks if the getType() and getName() methods both return "memfile".
@@ -142,7 +163,8 @@ TEST_F(MemfileLeaseMgrTest, getLeaseFilePath) {
     EXPECT_EQ(pmap["leasefile6"],
               lease_mgr->getLeaseFilePath(Memfile_LeaseMgr::V6));
 
-    pmap["persist"] = "no";
+    pmap["persist4"] = "no";
+    pmap["persist6"] = "no";
     lease_mgr.reset(new Memfile_LeaseMgr(pmap));
     EXPECT_TRUE(lease_mgr->getLeaseFilePath(Memfile_LeaseMgr::V4).empty());
     EXPECT_TRUE(lease_mgr->getLeaseFilePath(Memfile_LeaseMgr::V6).empty());



More information about the bind10-changes mailing list