BIND 10 trac2573, updated. 0f244147145019810ae16b36383352cbf4672919 [2573] Merge branch 'trac2573' of ssh://git.bind10.isc.org/var/bind10/git/bind10 into trac2573
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Jan 17 21:35:51 UTC 2013
The branch, trac2573 has been updated
via 0f244147145019810ae16b36383352cbf4672919 (commit)
via 9423db0d25e30b0a3e1334b56c0ae754675bb2ce (commit)
via fe931e2f1c65e3bde969aa3bb0a2a031201fab95 (commit)
from 24e0b47e82d29460f4dabfb66558204db9b3b83d (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 0f244147145019810ae16b36383352cbf4672919
Merge: 9423db0 24e0b47
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Thu Jan 17 13:35:02 2013 -0800
[2573] Merge branch 'trac2573' of ssh://git.bind10.isc.org/var/bind10/git/bind10 into trac2573
commit 9423db0d25e30b0a3e1334b56c0ae754675bb2ce
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Thu Jan 17 13:34:11 2013 -0800
[2573] reordered params to internal addRR of zone loader implementation.
it's just a matter of preference; I don't have a particular one.
commit fe931e2f1c65e3bde969aa3bb0a2a031201fab95
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Thu Jan 17 13:31:38 2013 -0800
[2573] return the original double of progress, instead of convert to int.
so the caller can get highest possible precission.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/tests/zone_loader_unittest.cc | 16 +++++++-------
src/lib/datasrc/zone_loader.cc | 17 ++++++++-------
src/lib/datasrc/zone_loader.h | 22 ++++++++++----------
src/lib/python/isc/datasrc/datasrc.cc | 2 +-
.../python/isc/datasrc/tests/zone_loader_test.py | 14 +++++++++----
src/lib/python/isc/datasrc/zone_loader_inc.cc | 22 +++++++++++---------
src/lib/python/isc/datasrc/zone_loader_python.cc | 2 +-
7 files changed, 53 insertions(+), 42 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/tests/zone_loader_unittest.cc b/src/lib/datasrc/tests/zone_loader_unittest.cc
index 0be33b1..4a244fc 100644
--- a/src/lib/datasrc/tests/zone_loader_unittest.cc
+++ b/src/lib/datasrc/tests/zone_loader_unittest.cc
@@ -323,9 +323,9 @@ TEST_F(ZoneLoaderTest, loadUnsigned) {
EXPECT_EQ(34, destination_client_.rrsets_.size());
// getRRCount should be identical of the RRs we've seen. progress
- // should reach 100%.
+ // should reach 100% (= 1).
EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
- EXPECT_EQ(100, loader.getProgress());
+ EXPECT_EQ(1, loader.getProgress());
// Ensure known order.
std::sort(destination_client_.rrsets_.begin(),
@@ -364,11 +364,13 @@ TEST_F(ZoneLoaderTest, loadUnsignedIncremental) {
// 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.). This also involves floating point operation, which may
- // cause a margin error depending on environments. If it happens we
- // should loosen the test condition to accept some margin.
+ // 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());
- EXPECT_EQ(27, loader.getProgress()); // file size = 1541, offset = 428
+ // 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);
@@ -377,7 +379,7 @@ TEST_F(ZoneLoaderTest, loadUnsignedIncremental) {
// Counters are updated accordingly. Progress should reach 100%.
EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
- EXPECT_EQ(100, loader.getProgress());
+ EXPECT_EQ(1, loader.getProgress());
// No more loading now
EXPECT_THROW(loader.load(), isc::InvalidOperation);
diff --git a/src/lib/datasrc/zone_loader.cc b/src/lib/datasrc/zone_loader.cc
index 6e22ed2..3f1107f 100644
--- a/src/lib/datasrc/zone_loader.cc
+++ b/src/lib/datasrc/zone_loader.cc
@@ -35,7 +35,7 @@ using isc::dns::MasterLexer;
namespace isc {
namespace datasrc {
-const int ZoneLoader::PROGRESS_UNKNOWN = -1;
+const double ZoneLoader::PROGRESS_UNKNOWN = -1;
ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
DataSourceClient& source) :
@@ -67,10 +67,10 @@ ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
namespace {
// Unified callback to install RR and increment RR count at the same time.
void
-addRR(const dns::Name& name, const dns::RRClass& rrclass,
+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, ZoneUpdater* updater,
- size_t* rr_count)
+ const dns::rdata::RdataPtr& data)
{
isc::dns::BasicRRset rrset(name, rrclass, type, ttl);
rrset.addRdata(data);
@@ -96,8 +96,9 @@ ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
createMasterLoaderCallbacks(zone_name,
updater_->getFinder().getClass(),
&loaded_ok_),
- boost::bind(addRR, _1, _2, _3, _4, _5,
- updater_.get(), &rr_count_)));
+ boost::bind(addRR,
+ updater_.get(), &rr_count_,
+ _1, _2, _3, _4, _5)));
}
}
@@ -157,7 +158,7 @@ ZoneLoader::getRRCount() const {
return (rr_count_);
}
-int
+double
ZoneLoader::getProgress() const {
if (!loader_) {
return (PROGRESS_UNKNOWN);
@@ -180,7 +181,7 @@ ZoneLoader::getProgress() const {
return (PROGRESS_UNKNOWN);
}
- return ((static_cast<double>(pos) / total_size) * 100);
+ return (static_cast<double>(pos) / total_size);
}
} // end namespace datasrc
diff --git a/src/lib/datasrc/zone_loader.h b/src/lib/datasrc/zone_loader.h
index a60ae6b..2bcf220 100644
--- a/src/lib/datasrc/zone_loader.h
+++ b/src/lib/datasrc/zone_loader.h
@@ -150,23 +150,23 @@ public:
/// \throw None
size_t getRRCount() const;
- /// \brief Return the current progress of the loader in percentage.
+ /// \brief Return the current progress of the loader.
///
- /// This method returns the current estimated progress of loader in
- /// percentage; it's 0 before starting the load, and 100 at the
- /// completion, and a value between 0 and 100 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.
+ /// 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 100.
+ /// 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 100. It should also expect the possibility of getting
+ /// 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).
@@ -187,13 +187,13 @@ public:
/// source can provide the latter value efficiently.
///
/// \throw None
- int getProgress() const;
+ double getProgress() const;
/// \brief A special value for \c getProgress, meaning the progress is
/// unknown.
///
/// See the method description for details.
- static const int PROGRESS_UNKNOWN;
+ static const double PROGRESS_UNKNOWN;
private:
/// \brief The iterator used as source of data in case of the copy mode.
diff --git a/src/lib/python/isc/datasrc/datasrc.cc b/src/lib/python/isc/datasrc/datasrc.cc
index f737b3b..cfacc64 100644
--- a/src/lib/python/isc/datasrc/datasrc.cc
+++ b/src/lib/python/isc/datasrc/datasrc.cc
@@ -201,7 +201,7 @@ initModulePart_ZoneLoader(PyObject* mod) {
try {
installClassVariable(zone_loader_type, "PROGRESS_UNKNOWN",
- Py_BuildValue("i", ZoneLoader::PROGRESS_UNKNOWN));
+ Py_BuildValue("d", ZoneLoader::PROGRESS_UNKNOWN));
} catch (const std::exception& ex) {
const std::string ex_what =
"Unexpected failure in ZoneLoader initialization: " +
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 8caef78..62f67cd 100644
--- a/src/lib/python/isc/datasrc/tests/zone_loader_test.py
+++ b/src/lib/python/isc/datasrc/tests/zone_loader_test.py
@@ -117,9 +117,9 @@ class ZoneLoaderTests(unittest.TestCase):
self.check_load()
# Expected values are hardcoded, taken from the test zone file,
- # assuming it won't change too often. progress should reach 100%.
+ # assuming it won't change too often. progress should reach 100% (=1).
self.assertEqual(8, self.loader.get_rr_count())
- self.assertEqual(100, self.loader.get_progress())
+ self.assertEqual(1, self.loader.get_progress())
def test_load_from_client(self):
self.source_client = isc.datasrc.DataSourceClient('sqlite3',
@@ -144,10 +144,16 @@ class ZoneLoaderTests(unittest.TestCase):
self.check_zone_soa(ORIG_SOA_TXT)
# In case it's from a zone file, get_progress should be in the middle
- # of (0, 100). expected value is taken from the test zone file
+ # of (0, 1). expected value is taken from the test zone file
# (total size = 422, current position = 288)
if from_file:
- self.assertEqual(int(288 * 100 / 422), self.loader.get_progress())
+ # 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))
diff --git a/src/lib/python/isc/datasrc/zone_loader_inc.cc b/src/lib/python/isc/datasrc/zone_loader_inc.cc
index d9bb120..ae6fa3c 100644
--- a/src/lib/python/isc/datasrc/zone_loader_inc.cc
+++ b/src/lib/python/isc/datasrc/zone_loader_inc.cc
@@ -129,25 +129,27 @@ Exceptions:\n\
\n\
";
+// Modifications
+// - double => float
const char* const ZoneLoader_getProgress_doc = "\
-get_progress() -> int\n\
+get_progress() -> float\n\
\n\
-Return the current progress of the loader in percentage.\n\
+Return the current progress of the loader.\n\
\n\
-This method returns the current estimated progress of loader in\n\
-percentage; it's 0 before starting the load, and 100 at the\n\
-completion, and a value between 0 and 100 in the middle of loading.\n\
-It's an implementation detail how to calculate the progress, which may\n\
-vary depending on how the loader is constructed and may even be\n\
-impossible to detect effectively.\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 100.\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 100.\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\
diff --git a/src/lib/python/isc/datasrc/zone_loader_python.cc b/src/lib/python/isc/datasrc/zone_loader_python.cc
index 668184d..c1915cc 100644
--- a/src/lib/python/isc/datasrc/zone_loader_python.cc
+++ b/src/lib/python/isc/datasrc/zone_loader_python.cc
@@ -196,7 +196,7 @@ ZoneLoader_getRRCount(PyObject* po_self, PyObject*) {
PyObject*
ZoneLoader_getProgress(PyObject* po_self, PyObject*) {
s_ZoneLoader* self = static_cast<s_ZoneLoader*>(po_self);
- return (Py_BuildValue("i", self->cppobj->getProgress()));
+ return (Py_BuildValue("d", self->cppobj->getProgress()));
}
// This list contains the actual set of functions we have in
More information about the bind10-changes
mailing list