Parcourir la source

[master] Merge branch 'trac2851'

JINMEI Tatuya il y a 12 ans
Parent
commit
a3d4fe8a32

+ 9 - 0
src/bin/auth/datasrc_clients_mgr.h

@@ -639,12 +639,21 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::getZoneWriter(
                   AUTH_DATASRC_CLIENTS_BUILDER_LOAD_ZONE_NOCACHE)
             .arg(origin).arg(rrclass);
         break;                  // return NULL below
+    case datasrc::ConfigurableClientList::CACHE_NOT_WRITABLE:
+        // This is an internal error. Auth server should skip reloading zones
+        // on non writable caches.
+        isc_throw(InternalCommandError, "failed to load zone " << origin
+                  << "/" << rrclass << ": internal failure, in-memory cache "
+                  "is not writable");
     case datasrc::ConfigurableClientList::CACHE_DISABLED:
         // This is an internal error. Auth server must have the cache
         // enabled.
         isc_throw(InternalCommandError, "failed to load zone " << origin
                   << "/" << rrclass << ": internal failure, in-memory cache "
                   "is somehow disabled");
+    default:                    // other cases can really never happen
+        isc_throw(Unexpected, "Impossible result in getting data source "
+                  "ZoneWriter: " << writerpair.first);
     }
 
     return (boost::shared_ptr<datasrc::memory::ZoneWriter>());

+ 47 - 8
src/bin/auth/tests/datasrc_clients_builder_unittest.cc

@@ -406,6 +406,22 @@ TEST_F(DataSrcClientsBuilderTest,
     EXPECT_EQ(orig_lock_count + 1, map_mutex.lock_count);
     EXPECT_EQ(orig_unlock_count + 1, map_mutex.unlock_count);
 
+    // zone doesn't exist in the data source
+    const ConstElementPtr config_nozone(Element::fromJSON("{"
+        "\"IN\": [{"
+        "    \"type\": \"sqlite3\","
+        "    \"params\": {\"database_file\": \"" + test_db + "\"},"
+        "    \"cache-enable\": true,"
+        "    \"cache-zones\": [\"nosuchzone.example\"]"
+        "}]}"));
+    clients_map = configureDataSource(config_nozone);
+    EXPECT_THROW(
+        builder.handleCommand(
+            Command(LOADZONE, Element::fromJSON(
+                        "{\"class\": \"IN\","
+                        " \"origin\": \"nosuchzone.example\"}"))),
+        TestDataSrcClientsBuilder::InternalCommandError);
+
     // basically impossible case: in-memory cache is completely disabled.
     // In this implementation of manager-builder, this should never happen,
     // but it catches it like other configuration error and keeps going.
@@ -503,14 +519,6 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
             }, "");
     }
 
-    // zone doesn't exist in the data source
-    EXPECT_THROW(
-        builder.handleCommand(
-            Command(LOADZONE,
-                    Element::fromJSON(
-                        "{\"class\": \"IN\", \"origin\": \"xx\"}"))),
-        TestDataSrcClientsBuilder::InternalCommandError);
-
     // origin is bogus
     EXPECT_THROW(builder.handleCommand(
                      Command(LOADZONE,
@@ -524,4 +532,35 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
                  isc::data::TypeError);
 }
 
+// This works only if mapped memory segment is compiled.
+// Note also that this test case may fail as we make b10-auth more aware
+// of shared-memory cache.
+TEST_F(DataSrcClientsBuilderTest,
+#ifdef USE_SHARED_MEMORY
+       loadInNonWritableCache
+#else
+       DISABLED_loadInNonWritableCache
+#endif
+    )
+{
+    const ConstElementPtr config = Element::fromJSON(
+        "{"
+        "\"IN\": [{"
+        "   \"type\": \"MasterFiles\","
+        "   \"params\": {"
+        "       \"test1.example\": \"" +
+        std::string(TEST_DATA_BUILDDIR "/test1.zone.copied") + "\"},"
+        "   \"cache-enable\": true,"
+        "   \"cache-type\": \"mapped\""
+        "}]}");
+    clients_map = configureDataSource(config);
+
+    EXPECT_THROW(builder.handleCommand(
+                     Command(LOADZONE,
+                             Element::fromJSON(
+                                 "{\"origin\": \"test1.example\","
+                                 " \"class\": \"IN\"}"))),
+                 TestDataSrcClientsBuilder::InternalCommandError);
+}
+
 } // unnamed namespace

+ 1 - 1
src/lib/datasrc/cache_config.cc

@@ -177,7 +177,7 @@ CacheConfig::getLoadAction(const dns::RRClass& rrclass,
     assert(datasrc_client_);
 
     // If the specified zone name does not exist in our client of the source,
-    // DataSourceError is thrown, which is exactly the result what we
+    // NoSuchZone is thrown, which is exactly the result what we
     // want, so no need to handle it.
     ZoneIteratorPtr iterator(datasrc_client_->getIterator(zone_name));
     if (!iterator) {

+ 6 - 5
src/lib/datasrc/cache_config.h

@@ -157,12 +157,13 @@ public:
     /// It doesn't throw an exception in this case because the expected caller
     /// of this method would handle such a case internally.
     ///
-    /// \throw DataSourceError error happens in the underlying data source
-    /// storing the cache data.  Most commonly it's because the specified zone
-    /// doesn't exist there.
+    /// \throw NoSuchZone The specified zone doesn't exist in the
+    /// underlying data source storing the original data to be cached.
+    /// \throw DataSourceError Other, unexpected but possible error happens
+    /// in the underlying data source.
     /// \throw Unexpected Unexpected error happens in the underlying data
-    /// source storing the cache data.  This shouldn't happen as long as the
-    /// data source implementation meets the public API requirement.
+    /// source.  This shouldn't happen as long as the data source
+    /// implementation meets the public API requirement.
     ///
     /// \param rrclass The RR class of the zone
     /// \param zone_name The origin name of the zone

+ 5 - 3
src/lib/datasrc/client.h

@@ -201,9 +201,6 @@ public:
     /// This allows for traversing the whole zone. The returned object can
     /// provide the RRsets one by one.
     ///
-    /// This throws DataSourceError when the zone does not exist in the
-    /// datasource.
-    ///
     /// The default implementation throws isc::NotImplemented. This allows
     /// for easy and fast deployment of minimal custom data sources, where
     /// the user/implementer doesn't have to care about anything else but
@@ -214,6 +211,11 @@ public:
     /// It is not fixed if a concrete implementation of this method can throw
     /// anything else.
     ///
+    /// \throw NoSuchZone the zone does not exist in the datasource.
+    /// \throw Others Possibly implementation specific exceptions (it is
+    /// not fixed if a concrete implementation of this method can throw
+    /// anything else.)
+    ///
     /// \param name The name of zone apex to be traversed. It doesn't do
     ///     nearest match as findZone.
     /// \param separate_rrs If true, the iterator will return each RR as a

+ 70 - 54
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,13 +130,22 @@ 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.
+            // If cache is disabled, or the zone table segment is not (yet)
+            // writable,  we are done for this data source.
             // Otherwise load zones into the in-memory cache.
             if (!cache_conf->isEnabled()) {
                 continue;
             }
+            memory::ZoneTableSegment& zt_segment =
+                *new_data_sources.back().ztable_segment_;
+            if (!zt_segment.isWritable()) {
+                LOG_DEBUG(logger, DBGLVL_TRACE_BASIC,
+                          DATASRC_LIST_CACHE_PENDING).arg(datasrc_name);
+                continue;
+            }
+
             internal::CacheConfig::ConstZoneIterator end_of_zones =
                 cache_conf->end();
             for (internal::CacheConfig::ConstZoneIterator zone_it =
@@ -144,25 +154,23 @@ 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 DataSourceError&) {
-                    isc_throw(ConfigurationError, "Data source error for "
-                              "loading a zone (possibly non-existent) "
-                              << zname << "/" << rrclass_);
-                }
-                assert(load_action); // in this loop this should be always true
-                try {
-                    memory::ZoneWriter writer(
-                        *new_data_sources.back().ztable_segment_,
+                    const memory::LoadAction load_action =
+                        cache_conf->getLoadAction(rrclass_, zname);
+                    // in this loop this should be always true
+                    assert(load_action);
+                    memory::ZoneWriter writer(zt_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());
                 }
             }
         }
@@ -309,48 +317,56 @@ ConfigurableClientList::findInternal(MutableResult& candidate,
     // and the need_updater parameter is true, get the zone there.
 }
 
-// We still provide this method for backward compatibility. But to not have
-// duplicate code, it is a thin wrapper around getCachedZoneWriter only.
-ConfigurableClientList::ReloadResult
-ConfigurableClientList::reload(const Name& name) {
-    const ZoneWriterPair result(getCachedZoneWriter(name));
-    if (result.first != ZONE_SUCCESS) {
-        return (result.first);
-    }
-
-    assert(result.second);
-    result.second->load();
-    result.second->install();
-    result.second->cleanup();
-
-    return (ZONE_SUCCESS);
-}
-
 ConfigurableClientList::ZoneWriterPair
-ConfigurableClientList::getCachedZoneWriter(const Name& name) {
+ConfigurableClientList::getCachedZoneWriter(const Name& name,
+                                            const std::string& datasrc_name)
+{
     if (!allow_cache_) {
         return (ZoneWriterPair(CACHE_DISABLED, ZoneWriterPtr()));
     }
-    // Try to find the correct zone.
-    MutableResult result;
-    findInternal(result, name, true, true);
-    if (!result.finder) {
-        return (ZoneWriterPair(ZONE_NOT_FOUND, ZoneWriterPtr()));
-    }
 
+    // Find the data source from which the zone to be loaded into memory.
     // Then get the appropriate load action and create a zone writer.
-    // Note that getCacheConfig() must return non NULL in this module (only
-    // tests could set it to a bogus value).
-    const memory::LoadAction load_action =
-        result.info->getCacheConfig()->getLoadAction(rrclass_, name);
-    if (!load_action) {
-        return (ZoneWriterPair(ZONE_NOT_CACHED, ZoneWriterPtr()));
+    BOOST_FOREACH(const DataSourceInfo& info, data_sources_) {
+        if (!datasrc_name.empty() && datasrc_name != info.name_) {
+            continue;
+        }
+        // If there's an underlying "real" data source and it doesn't contain
+        // the given name, obviously we cannot load it.  If a specific data
+        // source is given by the name, search should stop here.
+        if (info.data_src_client_ &&
+            info.data_src_client_->findZone(name).code != result::SUCCESS) {
+            if (!datasrc_name.empty()) {
+                return (ZoneWriterPair(ZONE_NOT_FOUND, ZoneWriterPtr()));
+            }
+            continue;
+        }
+        // If the corresponding zone table segment is not (yet) writable,
+        // we cannot load at this time.
+        if (info.ztable_segment_ && !info.ztable_segment_->isWritable()) {
+            return (ZoneWriterPair(CACHE_NOT_WRITABLE, ZoneWriterPtr()));
+        }
+        // Note that getCacheConfig() must return non NULL in this module
+        // (only tests could set it to a bogus value).
+        const memory::LoadAction load_action =
+            info.getCacheConfig()->getLoadAction(rrclass_, name);
+        if (!load_action) {
+            return (ZoneWriterPair(ZONE_NOT_CACHED, ZoneWriterPtr()));
+        }
+        return (ZoneWriterPair(ZONE_SUCCESS,
+                               ZoneWriterPtr(
+                                   new memory::ZoneWriter(
+                                       *info.ztable_segment_,
+                                       load_action, name, rrclass_))));
+    }
+
+    // We can't find the specified zone.  If a specific data source was
+    // given, this means the given name of data source doesn't exist, so
+    // we report it so.
+    if (!datasrc_name.empty()) {
+        return (ZoneWriterPair(DATASRC_NOT_FOUND, ZoneWriterPtr()));
     }
-    return (ZoneWriterPair(ZONE_SUCCESS,
-                           ZoneWriterPtr(
-                               new memory::ZoneWriter(
-                                   *result.info->ztable_segment_,
-                                   load_action, name, rrclass_))));
+    return (ZoneWriterPair(ZONE_NOT_FOUND, ZoneWriterPtr()));
 }
 
 // NOTE: This function is not tested, it would be complicated. However, the

+ 40 - 40
src/lib/datasrc/client_list.h

@@ -339,50 +339,51 @@ public:
         return (configuration_);
     }
 
-    /// \brief Result of the reload() method.
-    enum ReloadResult {
-        CACHE_DISABLED,     ///< The cache is not enabled in this list.
-        ZONE_NOT_CACHED,    ///< Zone is served directly, not from cache
-                            ///  (including the case cache is disabled for
-                            ///  the specific data source).
-        ZONE_NOT_FOUND,     ///< Zone does not exist in this list.
-        ZONE_SUCCESS        ///< The zone was successfully reloaded or
-                            ///  the writer provided.
-    };
-
-    /// \brief Reloads a cached zone.
-    ///
-    /// This method finds a zone which is loaded into a cache and reloads it.
-    /// This may be used to renew the cache when the underlying data source
-    /// changes.
-    ///
-    /// \param zone The origin of the zone to reload.
-    /// \return A status if the command worked.
-    /// \throw DataSourceError or anything else that the data source
-    ///      containing the zone might throw is propagated.
-    /// \throw DataSourceError if something unexpected happens, like when
-    ///      the original data source no longer contains the cached zone.
-    ReloadResult reload(const dns::Name& zone);
-
 private:
     /// \brief Convenience type shortcut
     typedef boost::shared_ptr<memory::ZoneWriter> ZoneWriterPtr;
 public:
+    /// \brief Codes indicating in-memory cache status for a given zone name.
+    ///
+    /// This is used as a result of the getCachedZoneWriter() method.
+    enum CacheStatus {
+        CACHE_DISABLED,     ///< The cache is not enabled in this list.
+        ZONE_NOT_CACHED,    ///< Zone is not to be cached (including the case
+                            ///  where caching is disabled for the specific
+                            ///  data source).
+        ZONE_NOT_FOUND,     ///< Zone does not exist in this list.
+        CACHE_NOT_WRITABLE, ///< The cache is not writable (and zones can't
+                            ///  be loaded)
+        DATASRC_NOT_FOUND,  ///< Specific data source for load is specified
+                            ///  but it's not in the list
+        ZONE_SUCCESS        ///< Zone to be cached is successfully found and
+                            ///  is ready to be loaded
+    };
 
     /// \brief Return value of getCachedZoneWriter()
     ///
     /// A pair containing status and the zone writer, for the
     /// getCachedZoneWriter() method.
-    typedef std::pair<ReloadResult, ZoneWriterPtr> ZoneWriterPair;
-
-    /// \brief Return a zone writer that can be used to reload a zone.
-    ///
-    /// This looks up a cached copy of zone and returns the ZoneWriter
-    /// that can be used to reload the content of the zone. This can
-    /// be used instead of reload() -- reload() works synchronously, which
-    /// is not what is needed every time.
-    ///
-    /// \param zone The origin of the zone to reload.
+    typedef std::pair<CacheStatus, ZoneWriterPtr> ZoneWriterPair;
+
+    /// \brief Return a zone writer that can be used to (re)load a zone.
+    ///
+    /// By default this method identifies the first data source in the list
+    /// that should serve the zone of the given name, and returns a ZoneWriter
+    /// object that can be used to load the content of the zone, in a specific
+    /// way for that data source.
+    ///
+    /// If the optional \c datasrc_name parameter is provided with a non empty
+    /// string, this method only tries to load the specified zone into or with
+    /// the data source which has the given name, regardless where in the list
+    /// that data source is placed.  Even if the given name of zone doesn't
+    /// exist in the data source, other data sources are not searched and
+    /// this method simply returns ZONE_NOT_FOUND in the first element
+    /// of the pair.
+    ///
+    /// \param zone The origin of the zone to load.
+    /// \param datasrc_name If not empty, the name of the data source
+    /// to be used for loading the zone (see above).
     /// \return The result has two parts. The first one is a status describing
     ///     if it worked or not (and in case it didn't, also why). If the
     ///     status is ZONE_SUCCESS, the second part contains a shared pointer
@@ -390,9 +391,8 @@ public:
     ///     NULL.
     /// \throw DataSourceError or anything else that the data source
     ///      containing the zone might throw is propagated.
-    /// \throw DataSourceError if something unexpected happens, like when
-    ///      the original data source no longer contains the cached zone.
-    ZoneWriterPair getCachedZoneWriter(const dns::Name& zone);
+    ZoneWriterPair getCachedZoneWriter(const dns::Name& zone,
+                                       const std::string& datasrc_name = "");
 
     /// \brief Implementation of the ClientList::find.
     virtual FindResult find(const dns::Name& zone,
@@ -421,12 +421,12 @@ public:
         boost::shared_ptr<memory::ZoneTableSegment> ztable_segment_;
         std::string name_;
 
+        // cache_conf_ can be accessed only from this read-only getter,
+        // to protect its integrity as much as possible.
         const internal::CacheConfig* getCacheConfig() const {
             return (cache_conf_.get());
         }
     private:
-        // this is kept private for now.  When it needs to be accessed,
-        // we'll add a read-only getter method.
         boost::shared_ptr<internal::CacheConfig> cache_conf_;
     };
 

+ 1 - 1
src/lib/datasrc/database.cc

@@ -1233,7 +1233,7 @@ public:
         const pair<bool, int> zone(accessor_->getZone(zone_name.toText()));
         if (!zone.first) {
             // No such zone, can't continue
-            isc_throw(DataSourceError, "Zone " + zone_name.toText() +
+            isc_throw(NoSuchZone, "Zone " + zone_name.toText() +
                       " can not be iterated, because it doesn't exist "
                       "in this data source");
         }

+ 17 - 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
@@ -347,6 +356,13 @@ not contain RRs the requested type.  AN NXRRSET indication is returned.
 A debug message indicating that a query for the given name and RR type is being
 processed.
 
+% DATASRC_LIST_CACHE_PENDING in-memory cache for data source '%1' is not yet writable, pending load
+While (re)configuring data source clients, zone data of the shown data
+source cannot be loaded to in-memory cache at that point because the
+cache is not yet ready for writing.  This can happen for shared-memory
+type of cache, in which case the cache will be reset later, either
+by a higher level application or by a command from other module.
+
 % DATASRC_LIST_NOT_CACHED zones in data source %1 for class %2 not cached, cache disabled globally. Will not be available.
 The process disabled caching of RR data completely. However, this data source
 is provided from a master file and it can be served from memory cache only.
@@ -354,7 +370,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

+ 18 - 2
src/lib/datasrc/exceptions.h

@@ -20,8 +20,14 @@
 namespace isc {
 namespace datasrc {
 
-/// This exception represents Backend-independent errors relating to
-/// data source operations.
+/// \brief Top level exception related to data source.
+///
+/// This exception is the most generic form of exception for errors or
+/// unexpected events that can happen in the data source module.  In general,
+/// if an application needs to catch these conditions explicitly, it should
+/// catch more specific exceptions derived from this class; the severity
+/// of the conditions will vary very much, and such an application would
+/// normally like to behave differently depending on the severity.
 class DataSourceError : public Exception {
 public:
     DataSourceError(const char* file, size_t line, const char* what) :
@@ -40,6 +46,16 @@ public:
         DataSourceError(file, line, what) {}
 };
 
+/// \brief A specified zone does not exist in the specified data source.
+///
+/// This exception is thrown from methods that take a zone name and perform
+/// some action regarding that zone on the corresponding data source.
+class NoSuchZone : public DataSourceError {
+public:
+    NoSuchZone(const char* file, size_t line, const char* what) :
+        DataSourceError(file, line, what) {}
+};
+
 /// Base class for a number of exceptions that are thrown while working
 /// with zones.
 struct ZoneException : public Exception {

+ 1 - 1
src/lib/datasrc/memory/memory_client.cc

@@ -242,7 +242,7 @@ InMemoryClient::getIterator(const Name& name, bool separate_rrs) const {
     const ZoneTable* zone_table = ztable_segment_->getHeader().getTable();
     const ZoneTable::FindResult result(zone_table->findZone(name));
     if (result.code != result::SUCCESS) {
-        isc_throw(DataSourceError, "No such zone: " + name.toText());
+        isc_throw(NoSuchZone, "No such zone: " + name.toText());
     }
 
     return (ZoneIteratorPtr(new MemoryIterator(

+ 1 - 1
src/lib/datasrc/tests/cache_config_unittest.cc

@@ -281,7 +281,7 @@ TEST_F(CacheConfigTest, getLoadActionWithMock) {
     // Zone configured for the cache but doesn't exist in the underling data
     // source.
     EXPECT_THROW(cache_conf.getLoadAction(RRClass::IN(), Name("example.net")),
-                 DataSourceError);
+                 NoSuchZone);
 
     // buggy data source client: it returns a null pointer from getIterator.
     EXPECT_THROW(cache_conf.getLoadAction(RRClass::IN(), Name("null.org")),

+ 177 - 102
src/lib/datasrc/tests/client_list_unittest.cc

@@ -250,6 +250,9 @@ public:
         EXPECT_EQ(cache, list_->getDataSources()[index].cache_ !=
                   shared_ptr<InMemoryClient>());
     }
+    ConfigurableClientList::CacheStatus doReload(
+        const Name& origin, const string& datasrc_name = "");
+
     const RRClass rrclass_;
     shared_ptr<TestedList> list_;
     const ClientList::FindResult negative_result_;
@@ -668,17 +671,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("["
         "{"
@@ -688,7 +694,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("["
         "{"
@@ -698,7 +704,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("["
         "{"
@@ -707,7 +713,35 @@ 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);
+}
+
+// This test relies on the property of mapped type of cache.
+TEST_F(ListTest,
+#ifdef USE_SHARED_MEMORY
+       cacheInNonWritableSegment
+#else
+       DISABLED_cacheInNonWritableSegment
+#endif
+    )
+{
+    // Initializing data source with non writable zone table memory segment
+    // is possible.  Loading is just postponed
+    const ConstElementPtr elem(Element::fromJSON("["
+        "{"
+        "   \"type\": \"test_type\","
+        "   \"cache-enable\": true,"
+        "   \"cache-type\": \"mapped\","
+        "   \"cache-zones\": [\"example.org\"],"
+        "   \"params\": [\"example.org\"]"
+        "}]"));
+    list_->configure(elem, true); // no disruption
+    checkDS(0, "test_type", "[\"example.org\"]", true);
+    const shared_ptr<InMemoryClient> cache(list_->getDataSources()[0].cache_);
+
+    // Likewise, reload attempt will fail.
+    EXPECT_EQ(ConfigurableClientList::CACHE_NOT_WRITABLE,
+              doReload(Name("example.org")));
 }
 
 TEST_F(ListTest, masterFiles) {
@@ -829,31 +863,10 @@ TEST_F(ListTest, BadMasterFile) {
                    true);
 }
 
-// This allows us to test two versions of the reloading code
-// (One by calling reload(), one by obtaining a ZoneWriter and
-// playing with that). Once we deprecate reload(), we should revert this
-// change and not use typed tests any more.
-template<class UpdateType>
-class ReloadTest : public ListTest {
-public:
-    ConfigurableClientList::ReloadResult doReload(const Name& origin);
-};
-
-// Version with calling reload()
-class ReloadUpdateType {};
-template<>
-ConfigurableClientList::ReloadResult
-ReloadTest<ReloadUpdateType>::doReload(const Name& origin) {
-    return (list_->reload(origin));
-};
-
-// Version with the ZoneWriter
-class WriterUpdateType {};
-template<>
-ConfigurableClientList::ReloadResult
-ReloadTest<WriterUpdateType>::doReload(const Name& origin) {
+ConfigurableClientList::CacheStatus
+ListTest::doReload(const Name& origin, const string& datasrc_name) {
     ConfigurableClientList::ZoneWriterPair
-        result(list_->getCachedZoneWriter(origin));
+        result(list_->getCachedZoneWriter(origin, datasrc_name));
     if (result.first == ConfigurableClientList::ZONE_SUCCESS) {
         // Can't use ASSERT_NE here, it would want to return(), which
         // it can't in non-void function.
@@ -866,157 +879,172 @@ ReloadTest<WriterUpdateType>::doReload(const Name& origin) {
                 "but the writer is NULL";
         }
     } else {
-        EXPECT_EQ(static_cast<memory::ZoneWriter*>(NULL),
-                  result.second.get());
+        EXPECT_EQ(static_cast<memory::ZoneWriter*>(NULL), result.second.get());
     }
     return (result.first);
 }
 
-// Typedefs for the GTEST guts to make it work
-typedef ::testing::Types<ReloadUpdateType, WriterUpdateType> UpdateTypes;
-TYPED_TEST_CASE(ReloadTest, UpdateTypes);
-
 // Test we can reload a zone
-TYPED_TEST(ReloadTest, reloadSuccess) {
-    this->list_->configure(this->config_elem_zones_, true);
+TEST_F(ListTest, reloadSuccess) {
+    list_->configure(config_elem_zones_, true);
     const Name name("example.org");
-    this->prepareCache(0, name);
+    prepareCache(0, name);
     // The cache currently contains a tweaked version of zone, which
     // doesn't have "tstzonedata" A record.  So the lookup should result
     // in NXDOMAIN.
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
-              this->list_->find(name).finder_->
+              list_->find(name).finder_->
                   find(Name("tstzonedata").concatenate(name),
                        RRType::A())->code);
     // Now reload the full zone. It should be there now.
-    EXPECT_EQ(ConfigurableClientList::ZONE_SUCCESS, this->doReload(name));
+    EXPECT_EQ(ConfigurableClientList::ZONE_SUCCESS, doReload(name));
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              this->list_->find(name).finder_->
+              list_->find(name).finder_->
                   find(Name("tstzonedata").concatenate(name),
                        RRType::A())->code);
 }
 
 // The cache is not enabled. The load should be rejected.
-TYPED_TEST(ReloadTest, reloadNotAllowed) {
-    this->list_->configure(this->config_elem_zones_, false);
+TEST_F(ListTest, reloadNotAllowed) {
+    list_->configure(config_elem_zones_, false);
     const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the thing.
-    this->prepareCache(0, name);
+    prepareCache(0, name);
     // See the reloadSuccess test.  This should result in NXDOMAIN.
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
-              this->list_->find(name).finder_->
+              list_->find(name).finder_->
                   find(Name("tstzonedata").concatenate(name),
                        RRType::A())->code);
     // Now reload. It should reject it.
-    EXPECT_EQ(ConfigurableClientList::CACHE_DISABLED, this->doReload(name));
+    EXPECT_EQ(ConfigurableClientList::CACHE_DISABLED, doReload(name));
     // Nothing changed here
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
-              this->list_->find(name).finder_->
+              list_->find(name).finder_->
                   find(Name("tstzonedata").concatenate(name),
                        RRType::A())->code);
 }
 
 // Similar to the previous case, but the cache is disabled in config.
-TYPED_TEST(ReloadTest, reloadNotEnabled) {
-    this->list_->configure(this->config_elem_zones_, true);
+TEST_F(ListTest, reloadNotEnabled) {
+    list_->configure(config_elem_zones_, true);
     const Name name("example.org");
     // We put the cache, actually disabling it.
-    this->prepareCache(0, name, false);
+    prepareCache(0, name, false);
     // In this case we cannot really look up due to the limitation of
     // the mock implementation.  We only check reload fails.
-    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_CACHED, this->doReload(name));
+    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_CACHED, doReload(name));
 }
 
 // Test several cases when the zone does not exist
-TYPED_TEST(ReloadTest, reloadNoSuchZone) {
-    this->list_->configure(this->config_elem_zones_, true);
+TEST_F(ListTest, reloadNoSuchZone) {
+    list_->configure(config_elem_zones_, true);
     const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the
     // reload method, as that one looks at the real state of things, not
     // at the configuration.
-    this->prepareCache(0, Name("example.com"));
+    prepareCache(0, Name("example.com"));
     // Not in the data sources
     EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND,
-              this->doReload(Name("exmaple.cz")));
-    // Not cached
-    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND, this->doReload(name));
+              doReload(Name("exmaple.cz")));
+    // If it's not configured to be cached, it won't be reloaded.
+    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_CACHED, doReload(name));
     // Partial match
     EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND,
-              this->doReload(Name("sub.example.com")));
+              doReload(Name("sub.example.com")));
     // Nothing changed here - these zones don't exist
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
-              this->list_->find(name).dsrc_client_);
+              list_->find(name).dsrc_client_);
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
-              this->list_->find(Name("example.cz")).dsrc_client_);
+              list_->find(Name("example.cz")).dsrc_client_);
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
-              this->list_->find(Name("sub.example.com"), true).dsrc_client_);
+              list_->find(Name("sub.example.com"), true).dsrc_client_);
     // Not reloaded, so A record shouldn't be visible yet.
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
-              this->list_->find(Name("example.com")).finder_->
+              list_->find(Name("example.com")).finder_->
                   find(Name("tstzonedata.example.com"),
                        RRType::A())->code);
 }
 
-// Check we gracefuly throw an exception when a zone disappeared in
-// the underlying data source when we want to reload it
-TYPED_TEST(ReloadTest, reloadZoneGone) {
-    this->list_->configure(this->config_elem_zones_, true);
+// Check we gracefully reject reloading (i.e. no exception) when a zone
+// disappeared in the underlying data source when we want to reload it
+TEST_F(ListTest, reloadZoneGone) {
+    list_->configure(config_elem_zones_, true);
     const Name name("example.org");
     // We put in a cache for non-existent zone. This emulates being loaded
     // and then the zone disappearing. We prefill the cache, so we can check
     // it.
-    this->prepareCache(0, name);
+    prepareCache(0, name);
     // The (cached) zone contains zone's SOA
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              this->list_->find(name).finder_->find(name,
-                                                    RRType::SOA())->code);
+              list_->find(name).finder_->find(name, RRType::SOA())->code);
     // Remove the zone from the data source.
     static_cast<MockDataSourceClient*>(
-        this->list_->getDataSources()[0].data_src_client_)->eraseZone(name);
+        list_->getDataSources()[0].data_src_client_)->eraseZone(name);
 
-    // The zone is not there, so abort the reload.
-    EXPECT_THROW(this->doReload(name), DataSourceError);
+    // The zone is not there, so reload doesn't take place.
+    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND, doReload(name));
     // The (cached) zone is not hurt.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              this->list_->find(name).finder_->find(name,
-                                                    RRType::SOA())->code);
+              list_->find(name).finder_->find(name, RRType::SOA())->code);
+}
+
+TEST_F(ListTest, reloadNewZone) {
+    // Test the case where a zone to be cached originally doesn't exist
+    // in the underlying data source and is added later.  reload() will
+    // succeed once it's available in the data source.
+    const ConstElementPtr elem(Element::fromJSON("["
+        "{"
+        "   \"type\": \"test_type\","
+        "   \"cache-enable\": true,"
+        "   \"cache-zones\": [\"example.org\", \"example.com\"],"
+        "   \"params\": [\"example.org\"]"
+        "}]"));
+    list_->configure(elem, true);
+    checkDS(0, "test_type", "[\"example.org\"]", true); // no example.com
+
+    // We can't reload it either
+    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND,
+              doReload(Name("example.com")));
+
+    // If we add the zone, we can now reload it
+    EXPECT_TRUE(static_cast<MockDataSourceClient*>(
+                    list_->getDataSources()[0].data_src_client_)->
+                insertZone(Name("example.com")));
+    EXPECT_EQ(ConfigurableClientList::ZONE_SUCCESS,
+              doReload(Name("example.com")));
 }
 
 // The underlying data source throws. Check we don't modify the state.
-TYPED_TEST(ReloadTest, reloadZoneThrow) {
-    this->list_->configure(this->config_elem_zones_, true);
+TEST_F(ListTest, reloadZoneThrow) {
+    list_->configure(config_elem_zones_, true);
     const Name name("noiter.org");
-    this->prepareCache(0, name);
+    prepareCache(0, name);
     // The zone contains stuff now
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              this->list_->find(name).finder_->find(name,
-                                                    RRType::SOA())->code);
+              list_->find(name).finder_->find(name, RRType::SOA())->code);
     // The iterator throws, so abort the reload.
-    EXPECT_THROW(this->doReload(name), isc::NotImplemented);
+    EXPECT_THROW(doReload(name), isc::NotImplemented);
     // The zone is not hurt.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              this->list_->find(name).finder_->find(name,
-                                                    RRType::SOA())->code);
+              list_->find(name).finder_->find(name, RRType::SOA())->code);
 }
 
-TYPED_TEST(ReloadTest, reloadNullIterator) {
-    this->list_->configure(this->config_elem_zones_, true);
+TEST_F(ListTest, reloadNullIterator) {
+    list_->configure(config_elem_zones_, true);
     const Name name("null.org");
-    this->prepareCache(0, name);
+    prepareCache(0, name);
     // The zone contains stuff now
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              this->list_->find(name).finder_->find(name,
-                                                    RRType::SOA())->code);
+              list_->find(name).finder_->find(name, RRType::SOA())->code);
     // The iterator throws, so abort the reload.
-    EXPECT_THROW(this->doReload(name), isc::Unexpected);
+    EXPECT_THROW(doReload(name), isc::Unexpected);
     // The zone is not hurt.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              this->list_->find(name).finder_->find(name,
-                                                    RRType::SOA())->code);
+              list_->find(name).finder_->find(name, RRType::SOA())->code);
 }
 
 // Test we can reload the master files too (special-cased)
-TYPED_TEST(ReloadTest, reloadMasterFile) {
+TEST_F(ListTest, reloadMasterFile) {
     const char* const install_cmd = INSTALL_PROG " -c " TEST_DATA_DIR
         "/root.zone " TEST_DATA_BUILDDIR "/root.zone.copied";
     if (system(install_cmd) != 0) {
@@ -1034,21 +1062,68 @@ TYPED_TEST(ReloadTest, reloadMasterFile) {
         "       \".\": \"" TEST_DATA_BUILDDIR "/root.zone.copied\""
         "   }"
         "}]"));
-    this->list_->configure(elem, true);
+    list_->configure(elem, true);
     // Add a record that is not in the zone
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
-              this->list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
-                                                         RRType::TXT())->code);
+              list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
+                                                   RRType::TXT())->code);
     ofstream f;
     f.open(TEST_DATA_BUILDDIR "/root.zone.copied", ios::out | ios::app);
     f << "nosuchdomain.\t\t3600\tIN\tTXT\ttest" << std::endl;
     f.close();
     // Do the reload.
-    EXPECT_EQ(ConfigurableClientList::ZONE_SUCCESS, this->doReload(Name(".")));
+    EXPECT_EQ(ConfigurableClientList::ZONE_SUCCESS, doReload(Name(".")));
     // It is here now.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              this->list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
-                                                         RRType::TXT())->code);
+              list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
+                                                   RRType::TXT())->code);
+}
+
+TEST_F(ListTest, reloadByDataSourceName) {
+    // We use three data sources (and their clients).  2nd and 3rd have
+    // the same name of the zones.
+    const ConstElementPtr config_elem = Element::fromJSON(
+        "[{\"type\": \"test_type1\", \"params\": [\"example.org\"]},"
+        " {\"type\": \"test_type2\", \"params\": [\"example.com\"]},"
+        " {\"type\": \"test_type3\", \"params\": [\"example.com\"]}]");
+    list_->configure(config_elem, true);
+    // Prepare in-memory cache for the 1st and 2nd data sources.
+    prepareCache(0, Name("example.org"));
+    prepareCache(1, Name("example.com"));
+
+    // Normal case: both zone name and data source name matches.
+    // See the reloadSuccess test about the NXDOMAIN/SUCCESS checks.
+    EXPECT_EQ(ZoneFinder::NXDOMAIN,
+              list_->find(Name("tstzonedata.example.com")).finder_->
+              find(Name("tstzonedata.example.com"), RRType::A())->code);
+    EXPECT_EQ(ConfigurableClientList::ZONE_SUCCESS,
+              doReload(Name("example.com"), "test_type2"));
+    EXPECT_EQ(ZoneFinder::SUCCESS,
+              list_->find(Name("tstzonedata.example.com")).finder_->
+              find(Name("tstzonedata.example.com"), RRType::A())->code);
+
+    // The specified zone exists in the first entry of the list, but a
+    // different data source name is specified (in which the specified zone
+    // doesn't exist), so reloading should fail, and the cache status of the
+    // first data source shouldn't change.
+    EXPECT_EQ(ZoneFinder::NXDOMAIN,
+              list_->find(Name("tstzonedata.example.org")).finder_->
+              find(Name("tstzonedata.example.org"), RRType::A())->code);
+    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND,
+              doReload(Name("example.org"), "test_type2"));
+    EXPECT_EQ(ZoneFinder::NXDOMAIN,
+              list_->find(Name("tstzonedata.example.org")).finder_->
+              find(Name("tstzonedata.example.org"), RRType::A())->code);
+
+    // Likewise, if a specific data source is given, normal name matching
+    // isn't suppressed and the 3rd data source will be used.  There cache
+    // is disabled, so reload should fail due to "not cached".
+    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_CACHED,
+              doReload(Name("example.com"), "test_type3"));
+
+    // specified name of data source doesn't exist.
+    EXPECT_EQ(ConfigurableClientList::DATASRC_NOT_FOUND,
+              doReload(Name("example.org"), "test_type4"));
 }
 
 // Check the status holds data

+ 2 - 2
src/lib/datasrc/tests/database_unittest.cc

@@ -1417,7 +1417,7 @@ TEST(GenericDatabaseClientTest, noAccessorException) {
 
 // If the zone doesn't exist, exception is thrown
 TEST_P(DatabaseClientTest, noZoneIterator) {
-    EXPECT_THROW(client_->getIterator(Name("example.com")), DataSourceError);
+    EXPECT_THROW(client_->getIterator(Name("example.com")), NoSuchZone);
 }
 
 // If the zone doesn't exist and iteration is not implemented, it still throws
@@ -1427,7 +1427,7 @@ TEST(GenericDatabaseClientTest, noZoneNotImplementedIterator) {
                                 boost::shared_ptr<DatabaseAccessor>(
                                     new NopAccessor())).getIterator(
                                         Name("example.com")),
-                 DataSourceError);
+                 NoSuchZone);
 }
 
 TEST(GenericDatabaseClientTest, notImplementedIterator) {

+ 1 - 1
src/lib/datasrc/tests/memory/memory_client_unittest.cc

@@ -669,7 +669,7 @@ TEST_F(MemoryClientTest, getZoneCount) {
 
 TEST_F(MemoryClientTest, getIteratorForNonExistentZone) {
     // Zone "." doesn't exist
-    EXPECT_THROW(client_->getIterator(Name(".")), DataSourceError);
+    EXPECT_THROW(client_->getIterator(Name(".")), NoSuchZone);
 }
 
 TEST_F(MemoryClientTest, getIterator) {

+ 1 - 1
src/lib/datasrc/tests/mock_client.cc

@@ -187,7 +187,7 @@ MockDataSourceClient::getIterator(const Name& name, bool) const {
         if (result.code == isc::datasrc::result::SUCCESS) {
             return (ZoneIteratorPtr(new Iterator(name, have_a_)));
         } else {
-            isc_throw(DataSourceError, "No such zone");
+            isc_throw(NoSuchZone, "No such zone");
         }
     }
 }

+ 7 - 0
src/lib/datasrc/tests/mock_client.h

@@ -58,6 +58,13 @@ public:
     void eraseZone(const dns::Name& zone_name) {
         zones.erase(zone_name);
     }
+
+    /// \brief Dynamically add a zone to the data source.
+    ///
+    /// \return true if the zone is newly added; false if it already exists.
+    bool insertZone(const dns::Name& zone_name) {
+        return (zones.insert(zone_name).second);
+    }
     const std::string type_;
     const data::ConstElementPtr configuration_;