[2204] extracted swapDataSrcClientLists() from configureDataSource().

now configureDataSource() can take time.

diff --git a/src/bin/auth/datasrc_config.cc b/src/bin/auth/datasrc_config.cc
index 73fb5190c6b885e2270659fe2f547c7598365612..bef6e57de71c2de184ef6307f5de9d6e0093643d 100644 (file)
--- a/src/bin/auth/datasrc_config.cc
+++ b/src/bin/auth/datasrc_config.cc
@@ -18,7 +18,7 @@
 
 // This is a trivial specialization for the commonly used version.
 // Defined in .cc to avoid accidental creation of multiple copies.
-void
+AuthSrv::DataSrcClientListsPtr
 configureDataSource(AuthSrv& server, const isc::data::ConstElementPtr& config)
 {
     return (configureDataSourceGeneric<AuthSrv,

diff --git a/src/bin/auth/datasrc_config.h b/src/bin/auth/datasrc_config.h
index 532aa34a2f3a5e797dc65d633052f2c06e3b9b13..9a6f7ac64a9608eef89ad1277a86d2a3b29be300 100644 (file)
--- a/src/bin/auth/datasrc_config.h
+++ b/src/bin/auth/datasrc_config.h
@@ -17,8 +17,6 @@
 
 #include "auth_srv.h"
 
-#include <util/threads/lock.h>
-
 #include <cc/data.h>
 #include <datasrc/client_list.h>
 
@@ -41,8 +39,9 @@
 /// \param config The configuration value to parse. It is in the form
 ///     as an update from the config manager.
 template<class Server, class List>
-void
-configureDataSourceGeneric(Server& server,
+boost::shared_ptr<std::map<isc::dns::RRClass,
+                           boost::shared_ptr<List> > > // = ListMap below
+configureDataSourceGeneric(Server& /*server*/,
                            const isc::data::ConstElementPtr& config)
 {
     typedef boost::shared_ptr<List> ListPtr;
@@ -63,18 +62,12 @@ configureDataSourceGeneric(Server& server,
                                                                 list));
     }
 
-    // Replace the server's lists.  By ignoring the return value we let the
-    // old lists be destroyed.  Lock will be released immediately after the
-    // swap.
-    {
-        isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
-        server.swapDataSrcClientLists(new_lists);
-    }
+    return (new_lists);
 }
 
 /// \brief Concrete version of configureDataSource() for the
 ///     use with authoritative server implementation.
-void
+AuthSrv::DataSrcClientListsPtr
 configureDataSource(AuthSrv& server, const isc::data::ConstElementPtr& config);
 
 #endif  // DATASRC_CONFIG_H

diff --git a/src/bin/auth/main.cc b/src/bin/auth/main.cc
index b425813006fe36c81efb1657b4b44d166388621d..2a17112ba4bed795231e12028f55228474177c36 100644 (file)
--- a/src/bin/auth/main.cc
+++ b/src/bin/auth/main.cc
@@ -18,6 +18,7 @@
 
 #include <util/buffer.h>
 #include <util/io/socketsession.h>
+#include <util/threads/lock.h>
 
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
@@ -93,18 +94,33 @@ datasrcConfigHandler(AuthSrv* server, bool* first_time,
 {
     assert(server != NULL);
     if (config->contains("classes")) {
+        AuthSrv::DataSrcClientListsPtr lists;
+
         if (*first_time) {
             // HACK: The default is not passed to the handler in the first
             // callback. This one will get the default (or, current value).
             // Further updates will work the usual way.
             assert(config_session != NULL);
             *first_time = false;
-            configureDataSource(*auth_server,
-                                config_session->getRemoteConfigValue(
-                                    "data_sources", "classes"));
+            lists = configureDataSource(
+                *auth_server,
+                config_session->getRemoteConfigValue("data_sources",
+                                                     "classes"));
         } else {
-            configureDataSource(*server, config->get("classes"));
+            lists = configureDataSource(*server, config->get("classes"));
+        }
+
+        // Replace the server's lists.  By ignoring the return value we let the
+        // old lists be destroyed.  Lock will be released immediately after the
+        // swap.
+        {
+            isc::util::thread::Mutex::Locker locker(
+                server->getClientListMutex());
+            lists = server->swapDataSrcClientLists(lists);
         }
+        // The previous lists are destroyed here.  Note that it's outside
+        // of the critical section protected by the locker.  So this can
+        // take time if running on a separate thread.
     }
 }
 

diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index dd60d896ad4628a7777da42aa556fc181a8d3ab5..f250d70f05a25488be4e6179bfc91e7b6a6d7f72 100644 (file)
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -725,6 +725,14 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) {
     EXPECT_FALSE(dnsserv.hasAnswer());
 }
 
+void
+installDataSrcClientLists(AuthSrv& server,
+                          AuthSrv::DataSrcClientListsPtr lists)
+{
+    thread::Mutex::Locker locker(server.getClientListMutex());
+    server.swapDataSrcClientLists(lists);
+}
+
 void
 updateDatabase(AuthSrv& server, const char* params) {
     const ConstElementPtr config(Element::fromJSON("{"
@@ -732,7 +740,7 @@ updateDatabase(AuthSrv& server, const char* params) {
         "    \"type\": \"sqlite3\","
         "    \"params\": " + string(params) +
         "}]}"));
-    configureDataSource(server, config);
+    installDataSrcClientLists(server, configureDataSource(server, config));
 }
 
 void
@@ -749,7 +757,7 @@ updateInMemory(AuthSrv& server, const char* origin, const char* filename) {
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    configureDataSource(server, config);
+    installDataSrcClientLists(server, configureDataSource(server, config));
 }
 
 void
@@ -759,7 +767,7 @@ updateBuiltin(AuthSrv& server) {
         "   \"type\": \"static\","
         "   \"params\": \"" + string(STATIC_DSRC_FILE) + "\""
         "}]}"));
-    configureDataSource(server, config);
+    installDataSrcClientLists(server, configureDataSource(server, config));
 }
 
 // Try giving the server a TSIG signed request and see it can anwer signed as
@@ -957,7 +965,7 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) {
         "   \"params\": {},"
         "   \"cache-enable\": true"
         "}]}"));
-    configureDataSource(server, config);
+    installDataSrcClientLists(server, configureDataSource(server, config));
     // after successful configuration, we should have one (with empty zoneset).
 
     // The memory data source is empty, should return REFUSED rcode.

diff --git a/src/bin/auth/tests/command_unittest.cc b/src/bin/auth/tests/command_unittest.cc
index b5e43edda20e79dbacad1b1d7246434394beaebb..36d13906b4d3630af4e9e67f9ccb24cb944774a6 100644 (file)
--- a/src/bin/auth/tests/command_unittest.cc
+++ b/src/bin/auth/tests/command_unittest.cc
@@ -191,6 +191,14 @@ zoneChecks(AuthSrv& server) {
               find(Name("ns.test2.example"), RRType::AAAA())->code);
 }
 
+void
+installDataSrcClientLists(AuthSrv& server,
+                          AuthSrv::DataSrcClientListsPtr lists)
+{
+    isc::util::thread::Mutex::Locker locker(server.getClientListMutex());
+    server.swapDataSrcClientLists(lists);
+}
+
 void
 configureZones(AuthSrv& server) {
     ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test1.zone.in "
@@ -210,7 +218,7 @@ configureZones(AuthSrv& server) {
         "   \"cache-enable\": true"
         "}]}"));
 
-    configureDataSource(server, config);
+    installDataSrcClientLists(server, configureDataSource(server, config));
 
     zoneChecks(server);
 }
@@ -273,7 +281,7 @@ TEST_F(AuthCommandTest,
         "    \"cache-enable\": true,"
         "    \"cache-zones\": [\"example.org\"]"
         "}]}"));
-    configureDataSource(server_, config);
+    installDataSrcClientLists(server_, configureDataSource(server_, config));
 
     {
         isc::util::thread::Mutex::Locker locker(server_.getClientListMutex());

diff --git a/src/bin/auth/tests/datasrc_config_unittest.cc b/src/bin/auth/tests/datasrc_config_unittest.cc
index 329c5d11beddbd0cd80f512fac63161a5bca690c..82067d30d8b646b034fa55051eccb69b37f61567 100644 (file)
--- a/src/bin/auth/tests/datasrc_config_unittest.cc
+++ b/src/bin/auth/tests/datasrc_config_unittest.cc
@@ -60,14 +60,11 @@ private:
 
 typedef shared_ptr<FakeList> ListPtr;
 
+// Forward declaration.  We need precise definition of DatasrcConfigTest
+// to complete this function.
 void
 testConfigureDataSource(DatasrcConfigTest& test,
-                        const isc::data::ConstElementPtr& config)
-{
-    // We use the test fixture for the Server type.  This makes it possible
-    // to easily fake all needed methods and look that they were called.
-    configureDataSourceGeneric<DatasrcConfigTest, FakeList>(test, config);
-}
+                        const isc::data::ConstElementPtr& config);
 
 void
 datasrcConfigHandler(DatasrcConfigTest* fake_server, const std::string&,
@@ -162,6 +159,17 @@ protected:
     mutable isc::util::thread::Mutex mutex_;
 };
 
+void
+testConfigureDataSource(DatasrcConfigTest& test,
+                        const isc::data::ConstElementPtr& config)
+{
+    // We use the test fixture for the Server type.  This makes it possible
+    // to easily fake all needed methods and look that they were called.
+    shared_ptr<std::map<dns::RRClass, ListPtr> > lists =
+        configureDataSourceGeneric<DatasrcConfigTest, FakeList>(test, config);
+    test.swapDataSrcClientLists(lists);
+}
+
 // Push there a configuration with a single list.
 TEST_F(DatasrcConfigTest, createList) {
     initializeINList();