Browse Source

[2851] don't make datasrc configure() fail in case of "NoSuchZone".

that should be more consistent with the case of missing zone file for
the MasterFiles data source.  and, it will be consistent with later
changes in this branch.
JINMEI Tatuya 12 years ago
parent
commit
2d3ed73912

+ 16 - 16
src/lib/datasrc/client_list.cc

@@ -102,10 +102,11 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
             }
             // Get the name (either explicit, or guess)
             const ConstElementPtr name_elem(dconf->get("name"));
-            const string name(name_elem ? name_elem->stringValue() : type);
-            if (!used_names.insert(name).second) {
+            const string datasrc_name =
+                name_elem ? name_elem->stringValue() : type;
+            if (!used_names.insert(datasrc_name).second) {
                 isc_throw(ConfigurationError, "Duplicate name in client list: "
-                          << name);
+                          << datasrc_name);
             }
 
             // Create a client for the underling data source via factory.
@@ -116,7 +117,7 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                                                                  paramConf);
             if (!allow_cache && !dsrc_pair.first) {
                 LOG_WARN(logger, DATASRC_LIST_NOT_CACHED).
-                    arg(name).arg(rrclass_);
+                    arg(datasrc_name).arg(rrclass_);
                 continue;
             }
 
@@ -129,7 +130,7 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
             new_data_sources.push_back(DataSourceInfo(dsrc_pair.first,
                                                       dsrc_pair.second,
                                                       cache_conf, rrclass_,
-                                                      name));
+                                                      datasrc_name));
 
             // If cache is disabled we are done for this data source.
             // Otherwise load zones into the in-memory cache.
@@ -144,25 +145,24 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                  ++zone_it)
             {
                 const Name& zname = zone_it->first;
-                memory::LoadAction load_action;
-                try {
-                    load_action = cache_conf->getLoadAction(rrclass_, zname);
-                } catch (const NoSuchZone&) {
-                    isc_throw(ConfigurationError, "Unable to cache "
-                              "non-existent zone: "
-                              << zname << "/" << rrclass_);
-                }
-                assert(load_action); // in this loop this should be always true
                 try {
+                    const memory::LoadAction load_action =
+                        cache_conf->getLoadAction(rrclass_, zname);
+                    // in this loop this should be always true
+                    assert(load_action);
                     memory::ZoneWriter writer(
                         *new_data_sources.back().ztable_segment_,
                         load_action, zname, rrclass_);
                     writer.load();
                     writer.install();
                     writer.cleanup();
+                } catch (const NoSuchZone&) {
+                    LOG_ERROR(logger, DATASRC_CACHE_ZONE_NOTFOUND).
+                        arg(zname).arg(rrclass_).arg(datasrc_name);
                 } catch (const ZoneLoaderException& e) {
-                    LOG_ERROR(logger, DATASRC_LOAD_ZONE_ERROR)
-                        .arg(zname).arg(rrclass_).arg(name).arg(e.what());
+                    LOG_ERROR(logger, DATASRC_LOAD_ZONE_ERROR).
+                        arg(zname).arg(rrclass_).arg(datasrc_name).
+                        arg(e.what());
                 }
             }
         }

+ 10 - 1
src/lib/datasrc/datasrc_messages.mes

@@ -70,6 +70,15 @@ The maximum allowed number of items of the hotspot cache is set to the given
 number. If there are too many, some of them will be dropped. The size of 0
 means no limit.
 
+% DATASRC_CACHE_ZONE_NOTFOUND Zone %1/%2 not found on data source '%3' to cache
+During data source configuration, a zone is to be loaded (cached) in
+to memory from the underlying data source, but the zone is not found
+in the data source.  This particular zone was not loaded, but data
+source configuration continues, possibly loading other zones into
+memory. This is basically some kind of configuration or operation
+error: either the name is incorrect in the configuration, or the zone
+has been removed from the data source without updating the configuration.
+
 % DATASRC_CHECK_ERROR post-load check of zone %1/%2 failed: %3
 The zone was loaded into the data source successfully, but the content fails
 basic sanity checks. See the message if you want to know what exactly is wrong
@@ -354,7 +363,7 @@ Therefore, the entire data source will not be available for this process. If
 this is a problem, you should configure the zones of that data source to some
 database backend (sqlite3, for example) and use it from there.
 
-% DATASRC_LOAD_ZONE_ERROR Error loading zone %1/%2 on data source %3: %4
+% DATASRC_LOAD_ZONE_ERROR Error loading zone %1/%2 on data source '%3': %4
 During data source configuration, an error was found in the zone data
 when it was being loaded in to memory on the shown data source.  This
 particular zone was not loaded, but data source configuration

+ 13 - 10
src/lib/datasrc/tests/client_list_unittest.cc

@@ -669,17 +669,20 @@ TEST_F(ListTest, cacheZones) {
 TEST_F(ListTest, badCache) {
     list_->configure(config_elem_, true);
     checkDS(0, "test_type", "{}", false);
-    // First, the zone is not in the data source
+    // First, the zone is not in the data source. configure() should still
+    // succeed, and the existence zone should be cached.
     const ConstElementPtr elem1(Element::fromJSON("["
         "{"
-        "   \"type\": \"type1\","
+        "   \"type\": \"test_type\","
         "   \"cache-enable\": true,"
-        "   \"cache-zones\": [\"example.org\"],"
-        "   \"params\": []"
+        "   \"cache-zones\": [\"example.org\", \"example.com\"],"
+        "   \"params\": [\"example.org\"]"
         "}]"));
-    EXPECT_THROW(list_->configure(elem1, true),
-                 ConfigurableClientList::ConfigurationError);
-    checkDS(0, "test_type", "{}", false);
+    list_->configure(elem1, true); // shouldn't cause disruption
+    checkDS(0, "test_type", "[\"example.org\"]", true);
+    const shared_ptr<InMemoryClient> cache(list_->getDataSources()[0].cache_);
+    EXPECT_EQ(1, cache->getZoneCount());
+    EXPECT_EQ(result::SUCCESS, cache->findZone(Name("example.org")).code);
     // Now, the zone doesn't give an iterator
     const ConstElementPtr elem2(Element::fromJSON("["
         "{"
@@ -689,7 +692,7 @@ TEST_F(ListTest, badCache) {
         "   \"params\": [\"noiter.org\"]"
         "}]"));
     EXPECT_THROW(list_->configure(elem2, true), isc::NotImplemented);
-    checkDS(0, "test_type", "{}", false);
+    checkDS(0, "test_type", "[\"example.org\"]", true);
     // Now, the zone returns NULL iterator
     const ConstElementPtr elem3(Element::fromJSON("["
         "{"
@@ -699,7 +702,7 @@ TEST_F(ListTest, badCache) {
         "   \"params\": [\"null.org\"]"
         "}]"));
     EXPECT_THROW(list_->configure(elem3, true), isc::Unexpected);
-    checkDS(0, "test_type", "{}", false);
+    checkDS(0, "test_type", "[\"example.org\"]", true);
     // The autodetection of zones is not enabled
     const ConstElementPtr elem4(Element::fromJSON("["
         "{"
@@ -708,7 +711,7 @@ TEST_F(ListTest, badCache) {
         "   \"params\": [\"example.org\"]"
         "}]"));
     EXPECT_THROW(list_->configure(elem4, true), isc::NotImplemented);
-    checkDS(0, "test_type", "{}", false);
+    checkDS(0, "test_type", "[\"example.org\"]", true);
 }
 
 TEST_F(ListTest, masterFiles) {