BIND 10 master, updated. 95d6795825d9df9634d2282b28d1d9a5761b6634 [master] Merge branch 'trac2573'

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Jan 18 17:53:56 UTC 2013


The branch, master has been updated
       via  95d6795825d9df9634d2282b28d1d9a5761b6634 (commit)
       via  0f244147145019810ae16b36383352cbf4672919 (commit)
       via  9423db0d25e30b0a3e1334b56c0ae754675bb2ce (commit)
       via  fe931e2f1c65e3bde969aa3bb0a2a031201fab95 (commit)
       via  24e0b47e82d29460f4dabfb66558204db9b3b83d (commit)
       via  2992f269ed62c67a5fb60ad60c0f7dcd89fba957 (commit)
       via  2f061fe969a7af07cecd9cd5d166f5b2d5b327aa (commit)
       via  6fbf5830da2a9ed728a865715bab0d59e1a5431b (commit)
       via  ae4b62b7f869cc7e66ef691ab20faff93566ac5b (commit)
       via  d7d8d1bf1a37b33891f09a2b3641c9bf5231d19d (commit)
       via  38439b178eeddb7148df3da5ebab555b294cc967 (commit)
       via  579f5abd033f56e772c6ee0286c58634d12aabfe (commit)
       via  867d9a39ee23b4e02ef9d36d241688f0ad525e5d (commit)
       via  1b32af4e5cb9349d34b468cd0333bbcfdb338d5a (commit)
      from  5204b8af562627419798146f71f7402849bfaedc (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 95d6795825d9df9634d2282b28d1d9a5761b6634
Merge: 5204b8a 0f24414
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Fri Jan 18 09:53:47 2013 -0800

    [master] Merge branch 'trac2573'
    
    fixed Conflicts:
    	src/lib/datasrc/zone_loader.cc

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

Summary of changes:
 src/lib/datasrc/tests/zone_loader_unittest.cc      |   46 +++++++++++++
 src/lib/datasrc/zone_loader.cc                     |   67 ++++++++++++++++---
 src/lib/datasrc/zone_loader.h                      |   69 ++++++++++++++++++--
 src/lib/dns/master_lexer.cc                        |   35 ++++++----
 src/lib/dns/master_lexer.h                         |   42 ++++++++----
 src/lib/dns/master_loader.cc                       |   19 +++++-
 src/lib/dns/master_loader.h                        |   42 ++++++++++++
 src/lib/dns/tests/master_lexer_unittest.cc         |   37 +++++++++--
 src/lib/dns/tests/master_loader_unittest.cc        |   69 +++++++++++++++++++-
 src/lib/python/isc/datasrc/datasrc.cc              |   19 ++++++
 .../python/isc/datasrc/tests/zone_loader_test.py   |   34 +++++++++-
 src/lib/python/isc/datasrc/zone_loader_inc.cc      |   60 +++++++++++++++++
 src/lib/python/isc/datasrc/zone_loader_python.cc   |   16 +++++
 13 files changed, 505 insertions(+), 50 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/tests/zone_loader_unittest.cc b/src/lib/datasrc/tests/zone_loader_unittest.cc
index f0ad814..28153e4 100644
--- a/src/lib/datasrc/tests/zone_loader_unittest.cc
+++ b/src/lib/datasrc/tests/zone_loader_unittest.cc
@@ -246,6 +246,11 @@ TEST_F(ZoneLoaderTest, copyUnsigned) {
     // It gets the updater directly in the constructor
     ASSERT_EQ(1, destination_client_.provided_updaters_.size());
     EXPECT_EQ(Name::ROOT_NAME(), destination_client_.provided_updaters_[0]);
+
+    // Counter is initialized to 0, progress is "unknown" in case of copy.
+    EXPECT_EQ(0, loader.getRRCount());
+    EXPECT_EQ(ZoneLoader::PROGRESS_UNKNOWN, loader.getProgress());
+
     // Now load the whole zone
     loader.load();
     EXPECT_TRUE(destination_client_.commit_called_);
@@ -254,6 +259,12 @@ TEST_F(ZoneLoaderTest, copyUnsigned) {
 
     // The count is 34 because we expect the RRs to be separated.
     EXPECT_EQ(34, destination_client_.rrsets_.size());
+
+    // Check various counters.  getRRCount should be identical of the RRs
+    // we've seen. Progress is still "unknown" in the copy operation.
+    EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
+    EXPECT_EQ(ZoneLoader::PROGRESS_UNKNOWN, loader.getProgress());
+
     // Ensure known order.
     std::sort(destination_client_.rrset_texts_.begin(),
               destination_client_.rrset_texts_.end());
@@ -281,6 +292,11 @@ TEST_F(ZoneLoaderTest, copyUnsignedIncremental) {
     // Not committed yet, we didn't complete the loading
     EXPECT_FALSE(destination_client_.commit_called_);
 
+    // Check we can get intermediate counters. Progress is always "unknown"
+    // in case of copy.
+    EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
+    EXPECT_EQ(ZoneLoader::PROGRESS_UNKNOWN, loader.getProgress());
+
     // This is unusual, but allowed. Check it doesn't do anything
     loader.loadIncremental(0);
     EXPECT_EQ(10, destination_client_.rrsets_.size());
@@ -349,6 +365,11 @@ TEST_F(ZoneLoaderTest, classMismatch) {
 TEST_F(ZoneLoaderTest, loadUnsigned) {
     ZoneLoader loader(destination_client_, Name::ROOT_NAME(),
                       TEST_DATA_DIR "/root.zone");
+
+    // Counter and progress are initialized to 0.
+    EXPECT_EQ(0, loader.getRRCount());
+    EXPECT_EQ(0, loader.getProgress());
+
     // It gets the updater directly in the constructor
     ASSERT_EQ(1, destination_client_.provided_updaters_.size());
     EXPECT_EQ(Name::ROOT_NAME(), destination_client_.provided_updaters_[0]);
@@ -360,6 +381,12 @@ TEST_F(ZoneLoaderTest, loadUnsigned) {
 
     // The count is 34 because we expect the RRs to be separated.
     EXPECT_EQ(34, destination_client_.rrsets_.size());
+
+    // getRRCount should be identical of the RRs we've seen.  progress
+    // should reach 100% (= 1).
+    EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
+    EXPECT_EQ(1, loader.getProgress());
+
     // Ensure known order.
     std::sort(destination_client_.rrset_texts_.begin(),
               destination_client_.rrset_texts_.end());
@@ -380,6 +407,10 @@ TEST_F(ZoneLoaderTest, loadUnsignedIncremental) {
     ZoneLoader loader(destination_client_, Name::ROOT_NAME(),
                       TEST_DATA_DIR "/root.zone");
 
+    // Counters are initialized to 0.
+    EXPECT_EQ(0, loader.getRRCount());
+    EXPECT_EQ(0, loader.getProgress());
+
     // Try loading few RRs first.
     loader.loadIncremental(10);
     // We should get the 10 we asked for
@@ -390,11 +421,26 @@ TEST_F(ZoneLoaderTest, loadUnsignedIncremental) {
     EXPECT_EQ(10, destination_client_.rrsets_.size());
     EXPECT_FALSE(destination_client_.commit_called_);
 
+    // Check we can get intermediate counters. Expected progress is calculated
+    // based on the size of the zone file and the offset to the end of 10th RR
+    // (subject to future changes to the file, but we assume it's a rare
+    // event.).  The expected value should be the exact expression that
+    // getProgress() should do internally, so EXPECT_EQ() should work here,
+    // but floating-point comparison can be always tricky we use
+    // EXPECT_DOUBLE_EQ just in case.
+    EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
+    // file size = 1541, offset = 428 (27.77%).
+    EXPECT_DOUBLE_EQ(static_cast<double>(428) / 1541, loader.getProgress());
+
     // We can finish the rest
     loader.loadIncremental(30);
     EXPECT_EQ(34, destination_client_.rrsets_.size());
     EXPECT_TRUE(destination_client_.commit_called_);
 
+    // Counters are updated accordingly. Progress should reach 100%.
+    EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
+    EXPECT_EQ(1, loader.getProgress());
+
     // No more loading now
     EXPECT_THROW(loader.load(), isc::InvalidOperation);
     EXPECT_THROW(loader.loadIncremental(1), isc::InvalidOperation);
diff --git a/src/lib/datasrc/zone_loader.cc b/src/lib/datasrc/zone_loader.cc
index 098b47a..9e9dd4a 100644
--- a/src/lib/datasrc/zone_loader.cc
+++ b/src/lib/datasrc/zone_loader.cc
@@ -34,17 +34,20 @@
 using isc::dns::Name;
 using isc::dns::ConstRRsetPtr;
 using isc::dns::MasterLoader;
+using isc::dns::MasterLexer;
 
 namespace isc {
 namespace datasrc {
 
+const double ZoneLoader::PROGRESS_UNKNOWN = -1;
+
 ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
                        DataSourceClient& source) :
     // Separate the RRsets as that is possibly faster (the data source doesn't
     // have to aggregate them) and also because our limit semantics.
     iterator_(source.getIterator(zone_name, true)),
     updater_(destination.getUpdater(zone_name, true, false)),
-    complete_(false)
+    complete_(false), rr_count_(0)
 {
     // The getIterator should never return NULL. So we check it.
     // Or should we throw instead?
@@ -65,11 +68,25 @@ ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
     }
 }
 
+namespace {
+// Unified callback to install RR and increment RR count at the same time.
+void
+addRR(ZoneUpdater* updater, size_t* rr_count,
+      const dns::Name& name, const dns::RRClass& rrclass,
+      const dns::RRType& type, const dns::RRTTL& ttl,
+      const dns::rdata::RdataPtr& data)
+{
+    isc::dns::BasicRRset rrset(name, rrclass, type, ttl);
+    rrset.addRdata(data);
+    updater->addRRset(rrset);
+    ++*rr_count;
+}
+}
+
 ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
                        const char* filename) :
     updater_(destination.getUpdater(zone_name, true, false)),
-    complete_(false),
-    loaded_ok_(true)
+    complete_(false), loaded_ok_(true), rr_count_(0)
 {
     if (updater_ == ZoneUpdaterPtr()) {
         isc_throw(DataSourceError, "Zone " << zone_name << " not found in "
@@ -83,7 +100,9 @@ ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
                                    createMasterLoaderCallbacks(zone_name,
                                        updater_->getFinder().getClass(),
                                        &loaded_ok_),
-                                   createMasterLoaderAddCallback(*updater_)));
+                                   boost::bind(addRR,
+                                               updater_.get(), &rr_count_,
+                                               _1, _2, _3, _4, _5)));
     }
 }
 
@@ -92,7 +111,7 @@ namespace {
 // Copy up to limit RRsets from source to destination
 bool
 copyRRsets(const ZoneUpdaterPtr& destination, const ZoneIteratorPtr& source,
-           size_t limit)
+           size_t limit, size_t& rr_count_)
 {
     size_t loaded = 0;
     while (loaded < limit) {
@@ -104,6 +123,7 @@ copyRRsets(const ZoneUpdaterPtr& destination, const ZoneIteratorPtr& source,
             destination->addRRset(*rrset);
         }
         ++loaded;
+        rr_count_ += rrset->getRdataCount();
     }
     return (false); // Not yet, there may be more
 }
@@ -133,8 +153,8 @@ ZoneLoader::loadIncremental(size_t limit) {
                   "Loading has been completed previously");
     }
 
-    if (iterator_ == ZoneIteratorPtr()) {
-        assert(loader_.get() != NULL);
+    if (!iterator_) {
+        assert(loader_);
         try {
             complete_ = loader_->loadIncremental(limit);
         } catch (const isc::dns::MasterLoaderError& e) {
@@ -144,7 +164,7 @@ ZoneLoader::loadIncremental(size_t limit) {
             isc_throw(MasterFileError, "Error while loading master file");
         }
     } else {
-        complete_ = copyRRsets(updater_, iterator_, limit);
+        complete_ = copyRRsets(updater_, iterator_, limit, rr_count_);
     }
 
     if (complete_) {
@@ -166,5 +186,36 @@ ZoneLoader::loadIncremental(size_t limit) {
     return (complete_);
 }
 
+size_t
+ZoneLoader::getRRCount() const {
+    return (rr_count_);
+}
+
+double
+ZoneLoader::getProgress() const {
+    if (!loader_) {
+        return (PROGRESS_UNKNOWN);
+    }
+
+    const size_t pos = loader_->getPosition();
+    const size_t total_size = loader_->getSize();
+
+    // If the current position is 0, progress should definitely be 0; we
+    // don't bother to check the total size even if it's "unknown".
+    if (pos == 0) {
+        return (0);
+    }
+
+    // These cases shouldn't happen with our usage of MasterLoader.  So, in
+    // theory, we could throw here; however, since this method is expected
+    // to be used for informational purposes only, that's probably too harsh.
+    // So we return "unknown" instead.
+    if (total_size == MasterLexer::SOURCE_SIZE_UNKNOWN || total_size == 0) {
+        return (PROGRESS_UNKNOWN);
+    }
+
+    return (static_cast<double>(pos) / total_size);
+}
+
 } // end namespace datasrc
 } // end namespace isc
diff --git a/src/lib/datasrc/zone_loader.h b/src/lib/datasrc/zone_loader.h
index c7e5ccd..2a4559e 100644
--- a/src/lib/datasrc/zone_loader.h
+++ b/src/lib/datasrc/zone_loader.h
@@ -154,10 +154,66 @@ public:
     /// \throw ZoneContentError when the zone doesn't pass sanity check.
     /// \note If the limit is exactly the number of RRs available to be loaded,
     ///     the method still returns false and true'll be returned on the next
-    ///     call (which will load 0 RRs). This is because the end of iterator or
-    ///     master file is detected when reading past the end, not when the last
-    ///     one is read.
+    ///     call (which will load 0 RRs). This is because the end of iterator
+    ///     or master file is detected when reading past the end, not when the
+    ///     last one is read.
     bool loadIncremental(size_t limit);
+
+    /// \brief Return the number of RRs loaded.
+    ///
+    /// This method returns the number of RRs loaded via this loader by the
+    /// time of the call.  Before starting the load it will return 0.
+    /// It will return the total number of RRs of the zone on and after
+    /// completing the load.
+    ///
+    /// \throw None
+    size_t getRRCount() const;
+
+    /// \brief Return the current progress of the loader.
+    ///
+    /// This method returns the current estimated progress of loader as a
+    /// value between 0 and 1 (inclusive); it's 0 before starting the load,
+    /// and 1 at the completion, and a value between these (exclusive) in the
+    /// middle of loading.  It's an implementation detail how to calculate
+    /// the progress, which may vary depending on how the loader is
+    /// constructed and may even be impossible to detect effectively.
+    ///
+    /// If the progress cannot be determined, this method returns a special
+    /// value of PROGRESS_UNKNOWN, which is not included in the range between
+    /// 0 and 1.
+    ///
+    /// As such, the application should use the return value only for
+    /// informational purposes such as logging.  For example, it shouldn't
+    /// be used to determine whether loading is completed by comparing it
+    /// to 1.  It should also expect the possibility of getting
+    /// \c PROGRESS_UNKNOWN at any call to this method; it shouldn't assume
+    /// the specific way of internal implementation as described below (which
+    /// is provided for informational purposes only).
+    ///
+    /// In this implementation, if the loader is constructed with a file
+    /// name, the progress value is measured by the number of characters
+    /// read from the zone file divided by the size of the zone file
+    /// (with taking into account any included files).  Note that due to
+    /// the possibility of intermediate included files, the total file size
+    /// cannot be fully fixed until the completion of the load.  And, due to
+    /// this possibility, return values from this method may not always
+    /// increase monotonically.
+    ///
+    /// If it's constructed with another data source client, this method
+    /// always returns \c PROGRESS_UNKNOWN; in future, however, it may become
+    /// possible to return something more useful, e.g, based on the result
+    /// of \c getRRCount() and the total number of RRs if the underlying data
+    /// source can provide the latter value efficiently.
+    ///
+    /// \throw None
+    double getProgress() const;
+
+    /// \brief A special value for \c getProgress, meaning the progress is
+    /// unknown.
+    ///
+    /// See the method description for details.
+    static const double PROGRESS_UNKNOWN;
+
 private:
     /// \brief The iterator used as source of data in case of the copy mode.
     const ZoneIteratorPtr iterator_;
@@ -169,9 +225,14 @@ private:
     bool complete_;
     /// \brief Was the loading successful?
     bool loaded_ok_;
+    size_t rr_count_;
 };
 
 }
 }
 
-#endif
+#endif  // DATASRC_ZONE_LOADER_H
+
+// Local Variables:
+// mode: c++
+// End:
diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc
index 7febd58..9563355 100644
--- a/src/lib/dns/master_lexer.cc
+++ b/src/lib/dns/master_lexer.cc
@@ -48,6 +48,7 @@ using namespace master_lexer_internal;
 
 struct MasterLexer::MasterLexerImpl {
     MasterLexerImpl() : source_(NULL), token_(MasterToken::NOT_STARTED),
+                        total_size_(0), popped_size_(0),
                         paren_count_(0), last_was_eol_(true),
                         has_previous_(false),
                         previous_paren_count_(0),
@@ -91,11 +92,28 @@ struct MasterLexer::MasterLexerImpl {
                 separators_.test(c & 0x7f));
     }
 
+    void setTotalSize() {
+        assert(source_ != NULL);
+        if (total_size_ != SOURCE_SIZE_UNKNOWN) {
+            const size_t current_size = source_->getSize();
+            if (current_size != SOURCE_SIZE_UNKNOWN) {
+                total_size_ += current_size;
+            } else {
+                total_size_ = SOURCE_SIZE_UNKNOWN;
+            }
+        }
+    }
+
     std::vector<InputSourcePtr> sources_;
     InputSource* source_;       // current source (NULL if sources_ is empty)
     MasterToken token_;         // currently recognized token (set by a state)
     std::vector<char> data_;    // placeholder for string data
 
+    // Keep track of the total size of all sources and characters that have
+    // been read from sources already popped.
+    size_t total_size_;         // accumulated size (# of chars) of sources
+    size_t popped_size_;        // total size of sources that have been popped
+
     // These are used in states, and defined here only as a placeholder.
     // The main lexer class does not need these members.
     size_t paren_count_;        // nest count of the parentheses
@@ -139,6 +157,7 @@ MasterLexer::pushSource(const char* filename, std::string* error) {
     impl_->source_ = impl_->sources_.back().get();
     impl_->has_previous_ = false;
     impl_->last_was_eol_ = true;
+    impl_->setTotalSize();
     return (true);
 }
 
@@ -154,6 +173,7 @@ MasterLexer::pushSource(std::istream& input) {
     impl_->source_ = impl_->sources_.back().get();
     impl_->has_previous_ = false;
     impl_->last_was_eol_ = true;
+    impl_->setTotalSize();
 }
 
 void
@@ -162,6 +182,7 @@ MasterLexer::popSource() {
         isc_throw(InvalidOperation,
                   "MasterLexer::popSource on an empty source");
     }
+    impl_->popped_size_ += impl_->source_->getPosition();
     impl_->sources_.pop_back();
     impl_->source_ = impl_->sources_.empty() ? NULL :
         impl_->sources_.back().get();
@@ -191,22 +212,12 @@ MasterLexer::getSourceLine() const {
 
 size_t
 MasterLexer::getTotalSourceSize() const {
-    size_t total_size = 0;
-    BOOST_FOREACH(InputSourcePtr& src, impl_->sources_) {
-        // If the size of any pushed source is unknown, the total is also
-        // considered unknown.
-        if (src->getSize() == SOURCE_SIZE_UNKNOWN) {
-            return (SOURCE_SIZE_UNKNOWN);
-        }
-
-        total_size += src->getSize();
-    }
-    return (total_size);
+    return (impl_->total_size_);
 }
 
 size_t
 MasterLexer::getPosition() const {
-    size_t position = 0;
+    size_t position = impl_->popped_size_;
     BOOST_FOREACH(InputSourcePtr& src, impl_->sources_) {
         position += src->getPosition();
     }
diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h
index d14febe..31c6443 100644
--- a/src/lib/dns/master_lexer.h
+++ b/src/lib/dns/master_lexer.h
@@ -477,7 +477,7 @@ public:
     ///
     /// This method returns the sum of the size of sources that have been
     /// pushed to the lexer by the time of the call.  It would give the
-    /// caller of some hint about the amount of data the lexer is working on.
+    /// caller some hint about the amount of data the lexer is working on.
     ///
     /// The size of a normal file is equal to the file size at the time of
     /// the source is pushed.  The size of other type of input stream is
@@ -488,25 +488,42 @@ public:
     /// stream is unknown.  It happens, for example, if the standard input
     /// is associated with a pipe from the output of another process and it's
     /// specified as an input source.  If the size of some of the pushed
-    /// pushed source is unknown, this method returns SOURCE_SIZE_UNKNOWN.
+    /// source is unknown, this method returns SOURCE_SIZE_UNKNOWN.
     ///
-    /// If there is no source pushed in the lexer, it returns 0.
+    /// The total size won't change when a source is popped.  So the return
+    /// values of this method will monotonically increase or
+    /// \c SOURCE_SIZE_UNKNOWN; once it returns \c SOURCE_SIZE_UNKNOWN,
+    /// any subsequent call will also result in that value, by the above
+    /// definition.
+    ///
+    /// Before pushing any source, it returns 0.
     ///
     /// \throw None
     size_t getTotalSourceSize() const;
 
-    /// \brief Return the position of lexer in the currently pushed sources.
+    /// \brief Return the position of lexer in the pushed sources so far.
     ///
     /// This method returns the position in terms of the number of recognized
-    /// characters from all sources.  Roughly speaking, the position in a
-    /// single source is the offset from the beginning of the file or stream
-    /// to the current "read cursor" of the lexer, and the return value of
-    /// this method is the sum of the position in all the pushed sources.
+    /// characters from all sources that have been pushed by the time of the
+    /// call.  Conceptually, the position in a single source is the offset
+    /// from the beginning of the file or stream to the current "read cursor"
+    /// of the lexer.  The return value of this method is the sum of the
+    /// positions in all the pushed sources.  If any of the sources has
+    /// already been popped, the position of the source at the time of the
+    /// pop operation will be used for the calculation.
     ///
     /// If the lexer reaches the end for each of all the pushed sources,
     /// the return value should be equal to that of \c getTotalSourceSize().
+    /// It's generally expected that a source is popped when the lexer
+    /// reaches the end of the source.  So, when the application of this
+    /// class parses all contents of all sources, possibly with multiple
+    /// pushes and pops, the return value of this method and
+    /// \c getTotalSourceSize() should be identical (unless the latter
+    /// returns SOURCE_SIZE_UNKNOWN).  But this is not necessarily
+    /// guaranteed as the application can pop a source in the middle of
+    /// parsing it.
     ///
-    /// If there is no source pushed in the lexer, it returns 0.
+    /// Before pushing any source, it returns 0.
     ///
     /// The return values of this method and \c getTotalSourceSize() would
     /// give the caller an idea of the progress of the lexer at the time of
@@ -515,11 +532,10 @@ public:
     /// this way may not make much sense; it can only give an informational
     /// hint of the progress.
     ///
-    /// Note also that if a source is popped, this method will normally return
-    /// a smaller number by definition (and so will \c getTotalSourceSize()).
-    /// Likewise, the conceptual "read cursor" would move backward after a
+    /// Note that the conceptual "read cursor" would move backward after a
     /// call to \c ungetToken(), in which case this method will return a
-    /// smaller value, too.
+    /// smaller value.  That is, unlike \c getTotalSourceSize(), return
+    /// values of this method may not always monotonically increase.
     ///
     /// \throw None
     size_t getPosition() const;
diff --git a/src/lib/dns/master_loader.cc b/src/lib/dns/master_loader.cc
index 400db43..29e7e1d 100644
--- a/src/lib/dns/master_loader.cc
+++ b/src/lib/dns/master_loader.cc
@@ -76,7 +76,8 @@ public:
         previous_name_(false),
         complete_(false),
         seen_error_(false),
-        warn_rfc1035_ttl_(true)
+        warn_rfc1035_ttl_(true),
+        rr_count_(0)
     {}
 
     void pushSource(const std::string& filename, const Name& current_origin) {
@@ -103,6 +104,9 @@ public:
 
     bool loadIncremental(size_t count_limit);
 
+    size_t getSize() const { return (lexer_.getTotalSourceSize()); }
+    size_t getPosition() const { return (lexer_.getPosition()); }
+
 private:
     void reportError(const std::string& filename, size_t line,
                      const std::string& reason)
@@ -418,12 +422,14 @@ private:
     vector<IncludeInfo> include_info_;
     bool previous_name_; // True if there was a previous name in this file
                          // (false at the beginning or after an $INCLUDE line)
+
 public:
     bool complete_;             // All work done.
     bool seen_error_;           // Was there at least one error during the
                                 // load?
     bool warn_rfc1035_ttl_;     // should warn if implicit TTL determination
                                 // from the previous RR is used.
+    size_t rr_count_;    // number of RRs successfully loaded
 };
 
 // A helper method of loadIncremental, parsing the first token of a new line.
@@ -554,6 +560,7 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
                               rdata);
                 // Good, we loaded another one
                 ++count;
+                ++rr_count_;
             } else {
                 seen_error_ = true;
                 if (!many_errors_) {
@@ -630,5 +637,15 @@ MasterLoader::loadedSucessfully() const {
     return (impl_->complete_ && !impl_->seen_error_);
 }
 
+size_t
+MasterLoader::getSize() const {
+    return (impl_->getSize());
+}
+
+size_t
+MasterLoader::getPosition() const {
+    return (impl_->getPosition());
+}
+
 } // end namespace dns
 } // end namespace isc
diff --git a/src/lib/dns/master_loader.h b/src/lib/dns/master_loader.h
index 9d8a755..37b2ef4 100644
--- a/src/lib/dns/master_loader.h
+++ b/src/lib/dns/master_loader.h
@@ -142,6 +142,44 @@ public:
     ///     finishing the load.
     bool loadedSucessfully() const;
 
+    /// \brief Return the total size of the zone files and streams.
+    ///
+    /// This method returns the size of the source of the zone to be loaded
+    /// (master zone files or streams) that is known at the time of the call.
+    /// For a zone file, it's the size of the file; for a stream, it's the
+    /// size of the data (in bytes) available at the start of the load.
+    /// If separate zone files are included via the $INCLUDE directive, the
+    /// sum of the sizes of these files are added.
+    ///
+    /// If the loader is constructed with a stream, the size can be
+    /// "unknown" as described for \c MasterLexer::getTotalSourceSize().
+    /// In this case this method always returns
+    /// \c MasterLexer::SOURCE_SIZE_UNKNOWN.
+    ///
+    /// If the loader is constructed with a zone file, this method
+    /// initially returns 0.  So until either \c load() or \c loadIncremental()
+    /// is called, the value is meaningless.
+    ///
+    /// Note that when the source includes separate files, this method
+    /// cannot take into account the included files that the loader has not
+    /// recognized at the time of call.  So it's possible that this method
+    /// returns different values at different times of call.
+    ///
+    /// \throw None
+    size_t getSize() const;
+
+    /// \brief Return the position of the loader in zone.
+    ///
+    /// This method returns a conceptual "position" of the loader in the
+    /// zone to be loaded.  Specifically, it returns the total number of
+    /// characters contained in the zone files and streams and recognized
+    /// by the loader.  Before starting the load it returns 0; on successful
+    /// completion it will be equal to the return value of \c getSize()
+    /// (unless the latter returns \c MasterLexer::SOURCE_SIZE_UNKNOWN).
+    ///
+    /// \throw None
+    size_t getPosition() const;
+
 private:
     class MasterLoaderImpl;
     MasterLoaderImpl* impl_;
@@ -151,3 +189,7 @@ private:
 } // end namespace isc
 
 #endif // MASTER_LOADER_H
+
+// Local Variables:
+// mode: c++
+// End:
diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc
index aececa7..039a0c3 100644
--- a/src/lib/dns/tests/master_lexer_unittest.cc
+++ b/src/lib/dns/tests/master_lexer_unittest.cc
@@ -52,7 +52,6 @@ void
 checkEmptySource(const MasterLexer& lexer) {
     EXPECT_TRUE(lexer.getSourceName().empty());
     EXPECT_EQ(0, lexer.getSourceLine());
-    EXPECT_EQ(0, lexer.getTotalSourceSize());
     EXPECT_EQ(0, lexer.getPosition());
 }
 
@@ -78,6 +77,7 @@ TEST_F(MasterLexerTest, pushStream) {
     lexer.popSource();
     EXPECT_EQ(0, lexer.getSourceCount());
     checkEmptySource(lexer);
+    EXPECT_EQ(4, lexer.getTotalSourceSize()); // this shouldn't change
 }
 
 TEST_F(MasterLexerTest, pushStreamFail) {
@@ -105,6 +105,7 @@ TEST_F(MasterLexerTest, pushFile) {
     lexer.popSource();
     checkEmptySource(lexer);
     EXPECT_EQ(0, lexer.getSourceCount());
+    EXPECT_EQ(143, lexer.getTotalSourceSize()); // this shouldn't change
 
     // If we give a non NULL string pointer, its content will be intact
     // if pushSource succeeds.
@@ -133,22 +134,44 @@ TEST_F(MasterLexerTest, pushFileFail) {
 }
 
 TEST_F(MasterLexerTest, nestedPush) {
-    ss << "test";
+    const string test_txt = "test";
+    ss << test_txt;
     lexer.pushSource(ss);
+
+    EXPECT_EQ(test_txt.size(), lexer.getTotalSourceSize());
+    EXPECT_EQ(0, lexer.getPosition());
+
     EXPECT_EQ(expected_stream_name, lexer.getSourceName());
 
+    // Read the string; getPosition() should reflect that.
+    EXPECT_EQ(MasterToken::STRING, lexer.getNextToken().getType());
+    EXPECT_EQ(test_txt.size(), lexer.getPosition());
+
     // We can push another source without popping the previous one.
     lexer.pushSource(TEST_DATA_SRCDIR "/masterload.txt");
     EXPECT_EQ(TEST_DATA_SRCDIR "/masterload.txt", lexer.getSourceName());
-    EXPECT_EQ(143 + 4, lexer.getTotalSourceSize()); // see above for magic nums
+    EXPECT_EQ(143 + test_txt.size(),
+              lexer.getTotalSourceSize()); // see above for magic nums
+
+    // the next token should be the EOL (skipping a comment line), its
+    // position in the file is 35 (hardcoded).
+    EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType());
+    EXPECT_EQ(test_txt.size() + 35, lexer.getPosition());
 
     // popSource() works on the "topmost" (last-pushed) source
     lexer.popSource();
     EXPECT_EQ(expected_stream_name, lexer.getSourceName());
-    EXPECT_EQ(4, lexer.getTotalSourceSize());
+
+    // pop shouldn't change the total size and the current position
+    EXPECT_EQ(143 + test_txt.size(), lexer.getTotalSourceSize());
+    EXPECT_EQ(test_txt.size() + 35, lexer.getPosition());
 
     lexer.popSource();
     EXPECT_TRUE(lexer.getSourceName().empty());
+
+    // size and position still shouldn't change
+    EXPECT_EQ(143 + test_txt.size(), lexer.getTotalSourceSize());
+    EXPECT_EQ(test_txt.size() + 35, lexer.getPosition());
 }
 
 TEST_F(MasterLexerTest, unknownSourceSize) {
@@ -164,9 +187,9 @@ TEST_F(MasterLexerTest, unknownSourceSize) {
     // Then the total size is also unknown.
     EXPECT_EQ(MasterLexer::SOURCE_SIZE_UNKNOWN, lexer.getTotalSourceSize());
 
-    // If we pop that source, the size becomes known again.
+    // Even if we pop that source, the size is still unknown.
     lexer.popSource();
-    EXPECT_EQ(4, lexer.getTotalSourceSize());
+    EXPECT_EQ(MasterLexer::SOURCE_SIZE_UNKNOWN, lexer.getTotalSourceSize());
 }
 
 TEST_F(MasterLexerTest, invalidPop) {
@@ -317,7 +340,7 @@ TEST_F(MasterLexerTest, includeAndInitialWS) {
     lexer.pushSource(ss2);
     EXPECT_EQ(MasterToken::INITIAL_WS,
               lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
-    EXPECT_EQ(2, lexer.getPosition()); // should be sum of position positions.
+    EXPECT_EQ(2, lexer.getPosition()); // should be sum of pushed positions.
 }
 
 // Test only one token can be ungotten
diff --git a/src/lib/dns/tests/master_loader_unittest.cc b/src/lib/dns/tests/master_loader_unittest.cc
index 051c662..636f73d 100644
--- a/src/lib/dns/tests/master_loader_unittest.cc
+++ b/src/lib/dns/tests/master_loader_unittest.cc
@@ -153,12 +153,23 @@ TEST_F(MasterLoaderTest, basicLoad) {
               RRClass::IN(), MasterLoader::MANY_ERRORS);
 
     EXPECT_FALSE(loader_->loadedSucessfully());
+
+    // The following three should be set to 0 initially in case the loader
+    // is constructed from a file name.
+    EXPECT_EQ(0, loader_->getSize());
+    EXPECT_EQ(0, loader_->getPosition());
+
     loader_->load();
     EXPECT_TRUE(loader_->loadedSucessfully());
 
     EXPECT_TRUE(errors_.empty());
     EXPECT_TRUE(warnings_.empty());
 
+    // Hardcode expected values taken from the test data file, assuming it
+    // won't change too often.
+    EXPECT_EQ(549, loader_->getSize());
+    EXPECT_EQ(549, loader_->getPosition());
+
     checkBasicRRs();
 }
 
@@ -195,6 +206,47 @@ TEST_F(MasterLoaderTest, include) {
     }
 }
 
+TEST_F(MasterLoaderTest, includeAndIncremental) {
+    // Check getSize() and getPosition() are adjusted before and after
+    // $INCLUDE.
+    const string first_rr = "before.example.org. 0 A 192.0.2.1\n";
+    const string include_str = "$INCLUDE " TEST_DATA_SRCDIR "/example.org";
+    const string zone_data = first_rr + include_str + "\n" +
+        "www 3600 IN AAAA 2001:db8::1\n";
+    stringstream ss(zone_data);
+    setLoader(ss, Name("example.org."), RRClass::IN(), MasterLoader::DEFAULT);
+
+    // On construction, getSize() returns the size of the data (exclude the
+    // the file to be included); position is set to 0.
+    EXPECT_EQ(zone_data.size(), loader_->getSize());
+    EXPECT_EQ(0, loader_->getPosition());
+
+    // Read the first RR.  getSize() doesn't change; position should be
+    // at the end of the first line.
+    loader_->loadIncremental(1);
+    EXPECT_EQ(zone_data.size(), loader_->getSize());
+    EXPECT_EQ(first_rr.size(), loader_->getPosition());
+
+    // Read next 4.  It includes $INCLUDE processing.  Magic number of 549
+    // is the size of the test zone file (see above); 506 is the position in
+    // the file at the end of 4th RR (due to extra comments it's smaller than
+    // the file size).
+    loader_->loadIncremental(4);
+    EXPECT_EQ(zone_data.size() + 549, loader_->getSize());
+    EXPECT_EQ(first_rr.size() + include_str.size() + 506,
+              loader_->getPosition());
+
+    // Read the last one.  At this point getSize and getPosition return
+    // the same value, indicating progress of 100%.
+    loader_->loadIncremental(1);
+    EXPECT_EQ(zone_data.size() + 549, loader_->getSize());
+    EXPECT_EQ(zone_data.size() + 549, loader_->getPosition());
+
+    // we were not interested in checking RRs in this test.  clear them to
+    // not confuse TearDown().
+    rrsets_.clear();
+}
+
 // A commonly used helper to check callback message.
 void
 checkCallbackMessage(const string& actual_msg, const string& expected_msg,
@@ -260,7 +312,7 @@ TEST_F(MasterLoaderTest, popAfterError) {
     const string include_str = "$include " TEST_DATA_SRCDIR
         "/broken.zone\nwww 3600 IN AAAA 2001:db8::1\n";
     stringstream ss(include_str);
-    // We don't test without MANY_ERRORS, we want to see what happens
+    // We perform the test with MANY_ERRORS, we want to see what happens
     // after the error.
     setLoader(ss, Name("example.org."), RRClass::IN(),
               MasterLoader::MANY_ERRORS);
@@ -277,11 +329,19 @@ TEST_F(MasterLoaderTest, popAfterError) {
 
 // Check it works the same when created based on a stream, not filename
 TEST_F(MasterLoaderTest, streamConstructor) {
-    stringstream zone_stream(prepareZone("", true));
+    const string zone_data(prepareZone("", true));
+    stringstream zone_stream(zone_data);
     setLoader(zone_stream, Name("example.org."), RRClass::IN(),
               MasterLoader::MANY_ERRORS);
 
     EXPECT_FALSE(loader_->loadedSucessfully());
+
+    // Unlike the basicLoad test, if we construct the loader from a stream
+    // getSize() returns the data size in the stream immediately after the
+    // construction.
+    EXPECT_EQ(zone_data.size(), loader_->getSize());
+    EXPECT_EQ(0, loader_->getPosition());
+
     loader_->load();
     EXPECT_TRUE(loader_->loadedSucessfully());
 
@@ -290,6 +350,11 @@ TEST_F(MasterLoaderTest, streamConstructor) {
     checkRR("example.org", RRType::SOA(), "ns1.example.org. "
             "admin.example.org. 1234 3600 1800 2419200 7200");
     checkRR("correct.example.org", RRType::A(), "192.0.2.2");
+
+    // On completion of the load, both getSize() and getPosition() return the
+    // size of the data.
+    EXPECT_EQ(zone_data.size(), loader_->getSize());
+    EXPECT_EQ(zone_data.size(), loader_->getPosition());
 }
 
 // Try loading data incrementally.
diff --git a/src/lib/python/isc/datasrc/datasrc.cc b/src/lib/python/isc/datasrc/datasrc.cc
index 37c9baa..cfacc64 100644
--- a/src/lib/python/isc/datasrc/datasrc.cc
+++ b/src/lib/python/isc/datasrc/datasrc.cc
@@ -21,6 +21,7 @@
 #include <datasrc/client.h>
 #include <datasrc/database.h>
 #include <datasrc/sqlite3_accessor.h>
+#include <datasrc/zone_loader.h>
 
 #include "datasrc.h"
 #include "client_python.h"
@@ -34,6 +35,9 @@
 #include <util/python/pycppwrapper_util.h>
 #include <dns/python/pydnspp_common.h>
 
+#include <stdexcept>
+#include <string>
+
 using namespace isc::datasrc;
 using namespace isc::datasrc::python;
 using namespace isc::util::python;
@@ -195,6 +199,21 @@ initModulePart_ZoneLoader(PyObject* mod) {
     }
     Py_INCREF(&zone_loader_type);
 
+    try {
+        installClassVariable(zone_loader_type, "PROGRESS_UNKNOWN",
+                             Py_BuildValue("d", ZoneLoader::PROGRESS_UNKNOWN));
+    } catch (const std::exception& ex) {
+        const std::string ex_what =
+            "Unexpected failure in ZoneLoader initialization: " +
+            std::string(ex.what());
+        PyErr_SetString(po_IscException, ex_what.c_str());
+        return (false);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError,
+                        "Unexpected failure in ZoneLoader initialization");
+        return (false);
+    }
+
     return (true);
 }
 
diff --git a/src/lib/python/isc/datasrc/tests/zone_loader_test.py b/src/lib/python/isc/datasrc/tests/zone_loader_test.py
index cb239ed..62f67cd 100644
--- a/src/lib/python/isc/datasrc/tests/zone_loader_test.py
+++ b/src/lib/python/isc/datasrc/tests/zone_loader_test.py
@@ -38,7 +38,7 @@ ORIG_SOA_TXT = 'example.com. 3600 IN SOA master.example.com. ' +\
                'admin.example.com. 1234 3600 1800 2419200 7200\n'
 NEW_SOA_TXT = 'example.com. 1000 IN SOA a.dns.example.com. ' +\
               'mail.example.com. 1 1 1 1 1\n'
-
+PROGRESS_UNKNOWN = isc.datasrc.ZoneLoader.PROGRESS_UNKNOWN
 
 class ZoneLoaderTests(unittest.TestCase):
     def setUp(self):
@@ -111,22 +111,50 @@ class ZoneLoaderTests(unittest.TestCase):
     def test_load_from_file(self):
         self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
                                              self.test_file)
+        self.assertEqual(0, self.loader.get_rr_count())
+        self.assertEqual(0, self.loader.get_progress())
+
         self.check_load()
 
+        # Expected values are hardcoded, taken from the test zone file,
+        # assuming it won't change too often.  progress should reach 100% (=1).
+        self.assertEqual(8, self.loader.get_rr_count())
+        self.assertEqual(1, self.loader.get_progress())
+
     def test_load_from_client(self):
         self.source_client = isc.datasrc.DataSourceClient('sqlite3',
                                     DB_SOURCE_CLIENT_CONFIG)
         self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
                                              self.source_client)
+
+        self.assertEqual(0, self.loader.get_rr_count())
+        self.assertEqual(PROGRESS_UNKNOWN, self.loader.get_progress())
+
         self.check_load()
 
-    def check_load_incremental(self):
+        # In case of loading from another data source, progress is unknown.
+        self.assertEqual(8, self.loader.get_rr_count())
+        self.assertEqual(PROGRESS_UNKNOWN, self.loader.get_progress())
+
+    def check_load_incremental(self, from_file=True):
         # New zone has 8 RRs
         # After 5, it should return False
         self.assertFalse(self.loader.load_incremental(5))
         # New zone should not have been loaded yet
         self.check_zone_soa(ORIG_SOA_TXT)
 
+        # In case it's from a zone file, get_progress should be in the middle
+        # of (0, 1).  expected value is taken from the test zone file
+        # (total size = 422, current position = 288)
+        if from_file:
+            # To avoid any false positive due to rounding errors, we convert
+            # them to near integers between 0 and 100.
+            self.assertEqual(int((288 * 100) / 422),
+                             int(self.loader.get_progress() * 100))
+            # Also check the return value has higher precision.
+            self.assertNotEqual(int(288 * 100 / 422),
+                                100 * self.loader.get_progress())
+
         # After 5 more, it should return True (only having read 3)
         self.assertTrue(self.loader.load_incremental(5))
         # New zone should now be loaded
@@ -147,7 +175,7 @@ class ZoneLoaderTests(unittest.TestCase):
                                             DB_SOURCE_CLIENT_CONFIG)
         self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
                                              self.source_client)
-        self.check_load_incremental()
+        self.check_load_incremental(False)
 
     def test_bad_file(self):
         self.check_zone_soa(ORIG_SOA_TXT)
diff --git a/src/lib/python/isc/datasrc/zone_loader_inc.cc b/src/lib/python/isc/datasrc/zone_loader_inc.cc
index 405ad1a..ae6fa3c 100644
--- a/src/lib/python/isc/datasrc/zone_loader_inc.cc
+++ b/src/lib/python/isc/datasrc/zone_loader_inc.cc
@@ -113,4 +113,64 @@ on the next call (which will load 0 RRs). This is because the end of\n\
 iterator or master file is detected when reading past the end, not\n\
 when the last one is read.\n\
 ";
+
+const char* const ZoneLoader_getRRCount_doc = "\
+get_rr_count() -> integer\n\
+\n\
+Return the number of RRs loaded.\n\
+\n\
+This method returns the number of RRs loaded via this loader by the\n\
+time of the call. Before starting the load it will return 0. It will\n\
+return the total number of RRs of the zone on and after completing the\n\
+load.\n\
+\n\
+Exceptions:\n\
+  None\n\
+\n\
+";
+
+// Modifications
+// - double => float
+const char* const ZoneLoader_getProgress_doc = "\
+get_progress() -> float\n\
+\n\
+Return the current progress of the loader.\n\
+\n\
+This method returns the current estimated progress of loader as a\n\
+value between 0 and 1 (inclusive); it's 0 before starting the load,\n\
+and 1 at the completion, and a value between these (exclusive) in the\n\
+middle of loading. It's an implementation detail how to calculate the\n\
+progress, which may vary depending on how the loader is constructed\n\
+and may even be impossible to detect effectively.\n\
+\n\
+If the progress cannot be determined, this method returns a special\n\
+value of PROGRESS_UNKNOWN, which is not included in the range between\n\
+0 and 1.\n\
+\n\
+As such, the application should use the return value only for\n\
+informational purposes such as logging. For example, it shouldn't be\n\
+used to determine whether loading is completed by comparing it to 1.\n\
+It should also expect the possibility of getting PROGRESS_UNKNOWN at\n\
+any call to this method; it shouldn't assume the specific way of\n\
+internal implementation as described below (which is provided for\n\
+informational purposes only).\n\
+\n\
+In this implementation, if the loader is constructed with a file name,\n\
+the progress value is measured by the number of characters read from\n\
+the zone file divided by the size of the zone file (with taking into\n\
+account any included files). Note that due to the possibility of\n\
+intermediate included files, the total file size cannot be fully fixed\n\
+until the completion of the load. And, due to this possibility, return\n\
+values from this method may not always increase monotonically.\n\
+\n\
+If it's constructed with another data source client, this method\n\
+always returns PROGRESS_UNKNOWN; in future, however, it may become\n\
+possible to return something more useful, e.g, based on the result of\n\
+get_rr_count() and the total number of RRs if the underlying data\n\
+source can provide the latter value efficiently.\n\
+\n\
+Exceptions:\n\
+  None\n\
+\n\
+";
 } // unnamed namespace
diff --git a/src/lib/python/isc/datasrc/zone_loader_python.cc b/src/lib/python/isc/datasrc/zone_loader_python.cc
index 98264b3..c1915cc 100644
--- a/src/lib/python/isc/datasrc/zone_loader_python.cc
+++ b/src/lib/python/isc/datasrc/zone_loader_python.cc
@@ -187,6 +187,18 @@ ZoneLoader_loadIncremental(PyObject* po_self, PyObject* args) {
     }
 }
 
+PyObject*
+ZoneLoader_getRRCount(PyObject* po_self, PyObject*) {
+    s_ZoneLoader* self = static_cast<s_ZoneLoader*>(po_self);
+    return (Py_BuildValue("I", self->cppobj->getRRCount()));
+}
+
+PyObject*
+ZoneLoader_getProgress(PyObject* po_self, PyObject*) {
+    s_ZoneLoader* self = static_cast<s_ZoneLoader*>(po_self);
+    return (Py_BuildValue("d", self->cppobj->getProgress()));
+}
+
 // This list contains the actual set of functions we have in
 // python. Each entry has
 // 1. Python method name
@@ -197,6 +209,10 @@ PyMethodDef ZoneLoader_methods[] = {
     { "load", ZoneLoader_load, METH_NOARGS, ZoneLoader_load_doc },
     { "load_incremental", ZoneLoader_loadIncremental, METH_VARARGS,
       ZoneLoader_loadIncremental_doc },
+    { "get_rr_count", ZoneLoader_getRRCount, METH_NOARGS,
+      ZoneLoader_getRRCount_doc },
+    { "get_progress", ZoneLoader_getProgress, METH_NOARGS,
+      ZoneLoader_getProgress_doc },
     { NULL, NULL, 0, NULL }
 };
 



More information about the bind10-changes mailing list