Browse Source

[2905] extended ZoneWriter::load() so it can set load error, use it clientlist.

this allows the caller to know whether a load error happens and log the
reason for it if it constructed the writer to catch load error exceptions
internally.  ConfigurableClientList::configure() needs it to log these
events.  the catch block in the method is now dead code and was removed.
JINMEI Tatuya 12 years ago
parent
commit
16dea270c9

+ 7 - 5
src/lib/datasrc/client_list.cc

@@ -163,16 +163,18 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                     // loading error and install an empty zone in the table.
                     // loading error and install an empty zone in the table.
                     memory::ZoneWriter writer(zt_segment, load_action, zname,
                     memory::ZoneWriter writer(zt_segment, load_action, zname,
                                               rrclass_, true);
                                               rrclass_, true);
-                    writer.load();
+
+                    std::string error_msg;
+                    writer.load(&error_msg);
+                    if (!error_msg.empty()) {
+                        LOG_ERROR(logger, DATASRC_LOAD_ZONE_ERROR).arg(zname).
+                            arg(rrclass_).arg(datasrc_name).arg(error_msg);
+                    }
                     writer.install();
                     writer.install();
                     writer.cleanup();
                     writer.cleanup();
                 } catch (const NoSuchZone&) {
                 } catch (const NoSuchZone&) {
                     LOG_ERROR(logger, DATASRC_CACHE_ZONE_NOTFOUND).
                     LOG_ERROR(logger, DATASRC_CACHE_ZONE_NOTFOUND).
                         arg(zname).arg(rrclass_).arg(datasrc_name);
                         arg(zname).arg(rrclass_).arg(datasrc_name);
-                } catch (const ZoneLoaderException& e) {
-                    LOG_ERROR(logger, DATASRC_LOAD_ZONE_ERROR).
-                        arg(zname).arg(rrclass_).arg(datasrc_name).
-                        arg(e.what());
                 }
                 }
             }
             }
         }
         }

+ 3 - 2
src/lib/datasrc/datasrc_messages.mes

@@ -374,8 +374,9 @@ database backend (sqlite3, for example) and use it from there.
 During data source configuration, an error was found in the zone data
 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
 when it was being loaded in to memory on the shown data source.  This
 particular zone was not loaded, but data source configuration
 particular zone was not loaded, but data source configuration
-continues, possibly loading other zones into memory. The specific
+continues, possibly loading other zones into memory.  Subsequent lookups
-error is shown in the message, and should be addressed.
+will treat this zone as broken.  The specific error is shown in the
+message, and should be addressed.
 
 
 % DATASRC_MASTER_LOAD_ERROR %1:%2: Zone '%3/%4' contains error: %5
 % DATASRC_MASTER_LOAD_ERROR %1:%2: Zone '%3/%4' contains error: %5
 There's an error in the given master file. The zone won't be loaded for
 There's an error in the given master file. The zone won't be loaded for

+ 4 - 1
src/lib/datasrc/memory/zone_writer.cc

@@ -52,7 +52,7 @@ ZoneWriter::~ZoneWriter() {
 }
 }
 
 
 void
 void
-ZoneWriter::load() {
+ZoneWriter::load(std::string* error_msg) {
     if (state_ != ZW_UNUSED) {
     if (state_ != ZW_UNUSED) {
         isc_throw(isc::InvalidOperation, "Trying to load twice");
         isc_throw(isc::InvalidOperation, "Trying to load twice");
     }
     }
@@ -70,6 +70,9 @@ ZoneWriter::load() {
         if (!catch_load_error_) {
         if (!catch_load_error_) {
             throw;
             throw;
         }
         }
+        if (error_msg) {
+            *error_msg = ex.what();
+        }
         segment_.resetHeader();
         segment_.resetHeader();
     }
     }
 
 

+ 10 - 1
src/lib/datasrc/memory/zone_writer.h

@@ -76,6 +76,12 @@ public:
     /// This is the first method you should call on the object. Never call it
     /// This is the first method you should call on the object. Never call it
     /// multiple times.
     /// multiple times.
     ///
     ///
+    /// If the optional parameter \c error_msg is given and non NULL, and
+    /// if the writer object was constructed with \c catch_load_error being
+    /// true, then error_msg will be filled with text indicating the reason
+    /// for the error in case a load error happens.  In other cases any
+    /// passed non NULL error_msg will be intact.
+    ///
     /// \note As this contains reading of files or other data sources, or with
     /// \note As this contains reading of files or other data sources, or with
     ///     some other source of the data to load, it may throw quite anything.
     ///     some other source of the data to load, it may throw quite anything.
     ///     If it throws, do not call any other methods on the object and
     ///     If it throws, do not call any other methods on the object and
@@ -85,7 +91,10 @@ public:
     /// \throw isc::InvalidOperation if called second time.
     /// \throw isc::InvalidOperation if called second time.
     /// \throw DataSourceError load related error (not thrown if constructed
     /// \throw DataSourceError load related error (not thrown if constructed
     /// with catch_load_error being false).
     /// with catch_load_error being false).
-    void load();
+    ///
+    /// \param error_msg If non NULL, used as a placeholder to store load error
+    /// messages.
+    void load(std::string* error_msg = NULL);
 
 
     /// \brief Put the changes to effect.
     /// \brief Put the changes to effect.
     ///
     ///

+ 27 - 0
src/lib/datasrc/tests/memory/zone_writer_unittest.cc

@@ -28,6 +28,8 @@
 #include <boost/scoped_ptr.hpp>
 #include <boost/scoped_ptr.hpp>
 #include <boost/bind.hpp>
 #include <boost/bind.hpp>
 
 
+#include <string>
+
 using boost::scoped_ptr;
 using boost::scoped_ptr;
 using boost::bind;
 using boost::bind;
 using isc::dns::RRClass;
 using isc::dns::RRClass;
@@ -252,9 +254,17 @@ TEST_F(ZoneWriterTest, loadThrows) {
 
 
 // Emulate the situation where load() throws loader error.
 // Emulate the situation where load() throws loader error.
 TEST_F(ZoneWriterTest, loadLoaderException) {
 TEST_F(ZoneWriterTest, loadLoaderException) {
+    std::string error_msg;
+
     // By default, the exception is propagated.
     // By default, the exception is propagated.
     load_loader_throw_ = true;
     load_loader_throw_ = true;
     EXPECT_THROW(writer_->load(), ZoneLoaderException);
     EXPECT_THROW(writer_->load(), ZoneLoaderException);
+    // In this case, passed error_msg won't be updated.
+    writer_.reset(new ZoneWriter(*segment_,
+                                 bind(&ZoneWriterTest::loadAction, this, _1),
+                                 Name("example.org"), RRClass::IN(), false));
+    EXPECT_THROW(writer_->load(&error_msg), ZoneLoaderException);
+    EXPECT_EQ("", error_msg);
 
 
     // If we specify allowing load error, load() will succeed and install()
     // If we specify allowing load error, load() will succeed and install()
     // adds an empty zone.
     // adds an empty zone.
@@ -272,6 +282,23 @@ TEST_F(ZoneWriterTest, loadLoaderException) {
     const ZoneTable::FindResult result = ztable->findZone(Name("example.org"));
     const ZoneTable::FindResult result = ztable->findZone(Name("example.org"));
     EXPECT_EQ(SUCCESS, result.code);
     EXPECT_EQ(SUCCESS, result.code);
     EXPECT_EQ(ZONE_EMPTY, result.flags);
     EXPECT_EQ(ZONE_EMPTY, result.flags);
+
+    // Allowing an error, and passing a template for the error message.
+    // It will be filled with the reason for the error.
+    writer_.reset(new ZoneWriter(*segment_,
+                                 bind(&ZoneWriterTest::loadAction, this, _1),
+                                 Name("example.org"), RRClass::IN(), true));
+    writer_->load(&error_msg);
+    EXPECT_NE("", error_msg);
+
+    // In case of no error, the placeholder will be intact.
+    load_loader_throw_ = false;
+    error_msg.clear();
+    writer_.reset(new ZoneWriter(*segment_,
+                                 bind(&ZoneWriterTest::loadAction, this, _1),
+                                 Name("example.org"), RRClass::IN(), true));
+    writer_->load(&error_msg);
+    EXPECT_EQ("", error_msg);
 }
 }
 
 
 // Check the strong exception guarantee - if it throws, nothing happened
 // Check the strong exception guarantee - if it throws, nothing happened