BIND 10 trac3075, updated. f004912dd236a53ee63ab51e937de7988681a22f [3075] Address review comments.
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Aug 26 18:29:13 UTC 2013
The branch, trac3075 has been updated
via f004912dd236a53ee63ab51e937de7988681a22f (commit)
from c39eb9bbe30285a2b19fea86473b63ddb758506b (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 f004912dd236a53ee63ab51e937de7988681a22f
Author: Thomas Markwalder <tmark at isc.org>
Date: Mon Aug 26 14:26:51 2013 -0400
[3075] Address review comments.
Minor corrections only, there were no major revisions.
-----------------------------------------------------------------------
Summary of changes:
src/bin/d2/d2_messages.mes | 11 +++-
src/bin/d2/d2_process.cc | 21 ++++---
src/bin/d2/d2_process.h | 45 ++++++++------
src/bin/d2/d2_queue_mgr.cc | 98 ++++++++++++++++--------------
src/bin/d2/d_process.h | 22 +++++--
src/bin/d2/dhcp-ddns.spec | 2 +-
src/bin/d2/tests/d2_cfg_mgr_unittests.cc | 3 +-
src/bin/d2/tests/d2_process_unittests.cc | 16 ++---
src/lib/dhcp_ddns/ncr_io.h | 2 +-
9 files changed, 128 insertions(+), 92 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/d2/d2_messages.mes b/src/bin/d2/d2_messages.mes
index 8701aa8..c2805fa 100644
--- a/src/bin/d2/d2_messages.mes
+++ b/src/bin/d2/d2_messages.mes
@@ -1,4 +1,3 @@
-/Users/tmark/ddns/build/new3075/bind10
# Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
#
# Permission to use, copy, modify, and/or distribute this software for any
@@ -173,7 +172,7 @@ needs to be increased, the DHCP-DDNS clients are simply generating too many
requests too quickly, or perhaps upstream DNS servers are experiencing
load issues.
-% DHCP_DDNS_QUEUE_MGR_RECONFIG application is reconfiguring the queue manager
+% DHCP_DDNS_QUEUE_MGR_RECONFIGURING application is reconfiguring the queue manager
This is an informational message indicating that DHCP_DDNS is reconfiguring the
queue manager as part of normal startup or in response to a new configuration.
@@ -199,7 +198,7 @@ manager if given a new configuration.
% DHCP_DDNS_QUEUE_MGR_RESUMING application is resuming listening for requests now that the request queue size has reached %1 of a maximum %2 allowed
This is an informational message indicating that DHCP_DDNS, which had stopped
-accpeting new requests, has processed enough entries from the receive queue to
+accepting new requests, has processed enough entries from the receive queue to
resume accepting requests.
% DHCP_DDNS_QUEUE_MGR_STARTED application's queue manager has begun listening for requests.
@@ -229,6 +228,12 @@ trying to stop the queue manager. This error is unlikely to occur or to
impair the application's ability to function but it should be reported for
analysis.
+% DHCP_DDNS_QUEUE_MGR_UNEXPECTED_HANDLER_ERROR application's queue manager request receive handler experienced an unexpected exception %1:
+This is an error message indicating that an unexpected error occurred within the
+DHCP_DDNS's Queue Manager request receive completion handler. This is most
+likely a programmatic issue that should be reported. The application may
+recover on its own.
+
% DHCP_DDNS_QUEUE_MGR_UNEXPECTED_STOP application's queue manager receive was
aborted unexpectedly while queue manager state is: %1
This is an error message indicating that DHCP_DDNS's Queue Manager request
diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc
index 181c833..09760d7 100644
--- a/src/bin/d2/d2_process.cc
+++ b/src/bin/d2/d2_process.cc
@@ -30,7 +30,7 @@ const char* D2Process::SD_INVALID_STR = "invalid";
// Setting to 80% for now. This is an arbitrary choice and should probably
// be configurable.
-const float D2Process::QUEUE_RESTART_PERCENT = 0.80;
+const unsigned int D2Process::QUEUE_RESTART_PERCENT = 80;
D2Process::D2Process(const char* name, IOServicePtr io_service)
: DProcessBase(name, io_service, DCfgMgrBasePtr(new D2CfgMgr())),
@@ -120,11 +120,11 @@ D2Process::runIO() {
cnt = asio_io_service.run_one();
}
- return cnt;
+ return (cnt);
}
bool
-D2Process::canShutdown() {
+D2Process::canShutdown() const {
bool all_clear = false;
// If we have been told to shutdown, find out if we are ready to do so.
@@ -151,6 +151,11 @@ D2Process::canShutdown() {
// Get out right now, no niceties.
all_clear = true;
break;
+
+ default:
+ // shutdown_type_ is an enum and should only be one of the above.
+ // if its getting through to this, something is whacked.
+ break;
}
if (all_clear) {
@@ -260,8 +265,8 @@ D2Process::checkQueueStatus() {
// Resume receiving once the queue has decreased by twenty
// percent. This is an arbitrary choice. @todo this value should
// probably be configurable.
- size_t threshold = (queue_mgr_->getMaxQueueSize()
- * QUEUE_RESTART_PERCENT);
+ size_t threshold = (((queue_mgr_->getMaxQueueSize()
+ * QUEUE_RESTART_PERCENT)) / 100);
if (queue_mgr_->getQueueSize() <= threshold) {
LOG_INFO (dctl_logger, DHCP_DDNS_QUEUE_MGR_RESUMING)
.arg(threshold).arg(queue_mgr_->getMaxQueueSize());
@@ -302,7 +307,7 @@ D2Process::checkQueueStatus() {
// we can do the reconfigure. In other words, we aren't RUNNING or
// STOPPING.
if (reconf_queue_flag_) {
- LOG_INFO (dctl_logger, DHCP_DDNS_QUEUE_MGR_RECONFIG);
+ LOG_INFO (dctl_logger, DHCP_DDNS_QUEUE_MGR_RECONFIGURING);
reconfigureQueueMgr();
}
break;
@@ -372,7 +377,7 @@ D2Process::getD2CfgMgr() {
return (boost::dynamic_pointer_cast<D2CfgMgr>(getCfgMgr()));
}
-const char* D2Process::getShutdownTypeStr(ShutdownType type) {
+const char* D2Process::getShutdownTypeStr(const ShutdownType& type) {
const char* str = SD_INVALID_STR;
switch (type) {
case SD_NORMAL:
@@ -384,6 +389,8 @@ const char* D2Process::getShutdownTypeStr(ShutdownType type) {
case SD_NOW:
str = SD_NOW_STR;
break;
+ default:
+ break;
}
return (str);
diff --git a/src/bin/d2/d2_process.h b/src/bin/d2/d2_process.h
index 5e0b203..f9c966c 100644
--- a/src/bin/d2/d2_process.h
+++ b/src/bin/d2/d2_process.h
@@ -33,6 +33,14 @@ class D2Process : public DProcessBase {
public:
/// @brief Defines the shutdown types supported by D2Process
+ ///
+ /// * SD_NORMAL - Stops the queue manager and finishes all current
+ /// transactions before exiting. This is the default.
+ ///
+ /// * SD_DRAIN_FIRST - Stops the queue manager but continues processing
+ /// requests from the queue until it is empty.
+ ///
+ /// * SD_NOW - Exits immediately.
enum ShutdownType {
SD_NORMAL,
SD_DRAIN_FIRST,
@@ -53,11 +61,11 @@ public:
/// state. Once the number of entries has decreased to this percentage
/// of the maximum allowed, D2Process will "resume" receiving requests
/// by restarting the queue manager.
- static const float QUEUE_RESTART_PERCENT;
+ static const unsigned int QUEUE_RESTART_PERCENT;
/// @brief Constructor
///
- /// The construction process creates the configuration manager, the queue
+ /// Construction creates the configuration manager, the queue
/// manager, and the update manager.
///
/// @param name name is a text label for the process. Generally used
@@ -71,11 +79,12 @@ public:
/// @brief Called after instantiation to perform initialization unique to
/// D2.
///
- /// This is called after command line arguments but PRIOR to configuration
- /// reception. The base class provides this method as a place to perform
- /// any derivation-specific initialization steps that are inapppropriate
- /// for the constructor but necessary prior to launch. So far, no such
- /// steps have been identified for D2, so its implementantion is empty.
+ /// This is invoked by the controller after command line arguments but
+ /// PRIOR to configuration reception. The base class provides this method
+ /// as a place to perform any derivation-specific initialization steps
+ /// that are inapppropriate for the constructor but necessary prior to
+ /// launch. So far, no such steps have been identified for D2, so its
+ /// implementantion is empty but required.
///
/// @throw DProcessBaseError if the initialization fails.
virtual void init();
@@ -84,7 +93,7 @@ public:
///
/// Once entered, the main control thread remains inside this method
/// until shutdown. The event loop logic is as follows:
- /// {{{
+ /// @code
/// while should not down {
/// process queue manager state change
/// process completed jobs
@@ -93,7 +102,7 @@ public:
///
/// ON an exception, exit with fatal error
/// }
- /// }}}
+ /// @endcode
///
/// To summarize, each pass through the event loop first checks the state
/// of the received queue and takes any steps required to ensure it is
@@ -233,7 +242,7 @@ protected:
/// If callbacks are ready to be executed upon entry, the method will
/// return as soon as these callbacks have completed. If no callbacks
/// are ready, then it will wait (indefinitely) until at least one callback
- /// is executed. (NOTE: Should become desirable to periodically force an
+ /// is executed. (@note: Should become desirable to periodically force an
/// event, an interval timer could be used to do so).
///
/// @return The number of callback handlers executed, or 0 if the IO
@@ -251,14 +260,14 @@ protected:
/// met.
///
/// @return Returns true if the criteria has been met, false otherwise.
- virtual bool canShutdown();
+ virtual bool canShutdown() const;
/// @brief Sets queue reconfigure indicator to the given value.
///
/// @param value is the new value to assign to the indicator
///
- /// NOTE this method is really only intended for testing purposes.
- void setReconfQueueFlag(bool value) {
+ /// @note this method is really only intended for testing purposes.
+ void setReconfQueueFlag(const bool value) {
reconf_queue_flag_ = value;
}
@@ -266,8 +275,8 @@ protected:
///
/// @param value is the new value to assign to shutdown type.
///
- /// NOTE this method is really only intended for testing purposes.
- void setShutdownType(ShutdownType value) {
+ /// @note this method is really only intended for testing purposes.
+ void setShutdownType(const ShutdownType& value) {
shutdown_type_ = value;
}
@@ -278,12 +287,12 @@ public:
D2CfgMgrPtr getD2CfgMgr();
/// @brief Returns a reference to the queue manager.
- D2QueueMgrPtr& getD2QueueMgr() {
+ const D2QueueMgrPtr& getD2QueueMgr() const {
return (queue_mgr_);
}
/// @brief Returns a reference to the update manager.
- D2UpdateMgrPtr& getD2UpdateMgr() {
+ const D2UpdateMgrPtr& getD2UpdateMgr() const {
return (update_mgr_);
}
@@ -305,7 +314,7 @@ public:
///
/// @return A text label corresponding the value or "invalid" if the
/// value is not a valid value.
- static const char* getShutdownTypeStr(ShutdownType type);
+ static const char* getShutdownTypeStr(const ShutdownType& type);
private:
/// @brief Pointer to our queue manager instance.
diff --git a/src/bin/d2/d2_queue_mgr.cc b/src/bin/d2/d2_queue_mgr.cc
index 91f56c2..4de9c42 100644
--- a/src/bin/d2/d2_queue_mgr.cc
+++ b/src/bin/d2/d2_queue_mgr.cc
@@ -36,54 +36,60 @@ D2QueueMgr::~D2QueueMgr() {
void
D2QueueMgr::operator()(const dhcp_ddns::NameChangeListener::Result result,
dhcp_ddns::NameChangeRequestPtr& ncr) {
- // Note that error conditions must be handled here without throwing
- // exceptions. Remember this is the application level "link" in the
- // callback chain. Throwing an exception here will "break" the
- // io_service "run" we are operating under. With that in mind,
- // if we hit a problem, we will stop the listener transition to
- // the appropriate stopped state. Upper layer(s) must monitor our
- // state as well as our queue size.
- switch (result) {
- case dhcp_ddns::NameChangeListener::SUCCESS:
- // Receive was successful, attempt to queue the request.
- if (getQueueSize() < getMaxQueueSize()) {
- // There's room on the queue, add to the end
- enqueue(ncr);
- return;
- }
-
- // Queue is full, stop the listener.
- // Note that we can move straight to a STOPPED state as there
- // is no receive in progress.
- LOG_ERROR(dctl_logger, DHCP_DDNS_QUEUE_MGR_QUEUE_FULL)
- .arg(max_queue_size_);
- stopListening(STOPPED_QUEUE_FULL);
- break;
-
- case dhcp_ddns::NameChangeListener::STOPPED:
- if (mgr_state_ == STOPPING) {
- // This is confirmation that the listener has stopped and its
- // callback will not be called again, unless its restarted.
- updateStopState();
- } else {
- // We should not get an receive complete status of stopped unless
- // we canceled the read as part of stopping. Therefore this is
- // unexpected so we will treat it as a receive error.
- // This is most likely an unforeseen programmatic issue.
- LOG_ERROR(dctl_logger, DHCP_DDNS_QUEUE_MGR_UNEXPECTED_STOP)
- .arg(mgr_state_);
+ try {
+ // Note that error conditions must be handled here without throwing
+ // exceptions. Remember this is the application level "link" in the
+ // callback chain. Throwing an exception here will "break" the
+ // io_service "run" we are operating under. With that in mind,
+ // if we hit a problem, we will stop the listener transition to
+ // the appropriate stopped state. Upper layer(s) must monitor our
+ // state as well as our queue size.
+ switch (result) {
+ case dhcp_ddns::NameChangeListener::SUCCESS:
+ // Receive was successful, attempt to queue the request.
+ if (getQueueSize() < getMaxQueueSize()) {
+ // There's room on the queue, add to the end
+ enqueue(ncr);
+ return;
+ }
+
+ // Queue is full, stop the listener.
+ // Note that we can move straight to a STOPPED state as there
+ // is no receive in progress.
+ LOG_ERROR(dctl_logger, DHCP_DDNS_QUEUE_MGR_QUEUE_FULL)
+ .arg(max_queue_size_);
+ stopListening(STOPPED_QUEUE_FULL);
+ break;
+
+ case dhcp_ddns::NameChangeListener::STOPPED:
+ if (mgr_state_ == STOPPING) {
+ // This is confirmation that the listener has stopped and its
+ // callback will not be called again, unless its restarted.
+ updateStopState();
+ } else {
+ // We should not get an receive complete status of stopped
+ // unless we canceled the read as part of stopping. Therefore
+ // this is unexpected so we will treat it as a receive error.
+ // This is most likely an unforeseen programmatic issue.
+ LOG_ERROR(dctl_logger, DHCP_DDNS_QUEUE_MGR_UNEXPECTED_STOP)
+ .arg(mgr_state_);
+ stopListening(STOPPED_RECV_ERROR);
+ }
+
+ break;
+
+ default:
+ // Receive failed, stop the listener.
+ // Note that we can move straight to a STOPPED state as there
+ // is no receive in progress.
+ LOG_ERROR(dctl_logger, DHCP_DDNS_QUEUE_MGR_RECV_ERROR);
stopListening(STOPPED_RECV_ERROR);
+ break;
}
-
- break;
-
- default:
- // Receive failed, stop the listener.
- // Note that we can move straight to a STOPPED state as there
- // is no receive in progress.
- LOG_ERROR(dctl_logger, DHCP_DDNS_QUEUE_MGR_RECV_ERROR);
- stopListening(STOPPED_RECV_ERROR);
- break;
+ } catch (const std::exception& ex) {
+ // On the outside chance a throw occurs, let's log it and swallow it.
+ LOG_ERROR(dctl_logger, DHCP_DDNS_QUEUE_MGR_UNEXPECTED_HANDLER_ERROR)
+ .arg(ex.what());
}
}
diff --git a/src/bin/d2/d_process.h b/src/bin/d2/d_process.h
index 9dd83f3..ce86118 100644
--- a/src/bin/d2/d_process.h
+++ b/src/bin/d2/d_process.h
@@ -35,12 +35,17 @@ public:
isc::Exception(file, line, what) { };
};
+/// @brief String value for the shutdown command.
+static const std::string SHUT_DOWN_COMMAND("shutdown");
+
+/// @brief Returned by the process to indicate a command was successful.
static const int COMMAND_SUCCESS = 0;
+
+/// @brief Returned by the process to indicates a command failed.
static const int COMMAND_ERROR = 1;
-static const int COMMAND_INVALID = 2;
-static const std::string SHUT_DOWN_COMMAND("shutdown");
-static const int CONFIG_INVALID = 1;
+/// @brief Returned by the process to indicates a command is not valid.
+static const int COMMAND_INVALID = 2;
/// @brief Application Process Interface
///
@@ -134,7 +139,12 @@ public:
/// @param args is a set of arguments (if any) required for the given
/// command.
/// @return an Element that contains the results of command composed
- /// of an integer status value (0 means successful, non-zero means failure),
+ /// of an integer status value:
+ ///
+ /// - COMMAND_SUCCESS indicates a command was successful.
+ /// - COMMAND_ERROR indicates a valid command failed execute.
+ /// - COMMAND_INVALID indicates a command is not valid.
+ ///
/// and a string explanation of the outcome.
virtual isc::data::ConstElementPtr command(
const std::string& command, isc::data::ConstElementPtr args) = 0;
@@ -145,7 +155,7 @@ public:
/// @brief Checks if the process has been instructed to shut down.
///
/// @return true if process shutdown flag is true.
- bool shouldShutdown() {
+ bool shouldShutdown() const {
return (shut_down_flag_);
}
@@ -158,7 +168,7 @@ public:
/// @brief Fetches the application name.
///
- /// @return a the application name string.
+ /// @return application name string.
const std::string getAppName() const {
return (app_name_);
}
diff --git a/src/bin/d2/dhcp-ddns.spec b/src/bin/d2/dhcp-ddns.spec
index 2925e5c..767f647 100644
--- a/src/bin/d2/dhcp-ddns.spec
+++ b/src/bin/d2/dhcp-ddns.spec
@@ -195,7 +195,7 @@
"commands": [
{
"command_name": "shutdown",
- "command_description": "Shuts down DHCP_DDNS server.",
+ "command_description": "Shuts down b1-dhcp-ddns module server.",
"command_args": [
{
"item_name": "type",
diff --git a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc
index 375e345..645bbae 100644
--- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc
+++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc
@@ -347,8 +347,7 @@ TEST_F(TSIGKeyInfoTest, invalidTSIGKeyList) {
ASSERT_NO_THROW(parser.reset(new TSIGKeyInfoListParser("test", keys_)));
// Verify that the list builds without errors.
- //ASSERT_NO_THROW(parser->build(config_set_));
- parser->build(config_set_);
+ ASSERT_NO_THROW(parser->build(config_set_));
// Verify that the list commit fails.
EXPECT_THROW(parser->commit(), D2CfgError);
diff --git a/src/bin/d2/tests/d2_process_unittests.cc b/src/bin/d2/tests/d2_process_unittests.cc
index bc509d3..4953ab6 100644
--- a/src/bin/d2/tests/d2_process_unittests.cc
+++ b/src/bin/d2/tests/d2_process_unittests.cc
@@ -113,7 +113,7 @@ public:
// Must call checkQueueStatus, to cause queue manager to reconfigure
// and start.
checkQueueStatus();
- D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
+ const D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
// If queue manager isn't in the RUNNING state, return failure.
if (D2QueueMgr::RUNNING != queue_mgr->getMgrState()) {
@@ -161,7 +161,7 @@ TEST(D2Process, construction) {
D2QueueMgrPtr queue_mgr = d2process.getD2QueueMgr();
ASSERT_TRUE(queue_mgr);
- D2UpdateMgrPtr& update_mgr = d2process.getD2UpdateMgr();
+ const D2UpdateMgrPtr& update_mgr = d2process.getD2UpdateMgr();
ASSERT_TRUE(update_mgr);
}
@@ -219,7 +219,7 @@ TEST_F(D2ProcessTest, configure) {
/// stop is initiated.
TEST_F(D2ProcessTest, queueStopOnShutdown) {
ASSERT_TRUE(runWithConfig(valid_d2_config));
- D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
+ const D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
setShutdownFlag(true);
@@ -247,7 +247,7 @@ TEST_F(D2ProcessTest, queueStopOnShutdown) {
/// manager stop is initiated.
TEST_F(D2ProcessTest, queueStopOnReconf) {
ASSERT_TRUE(runWithConfig(valid_d2_config));
- D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
+ const D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
// Manually set the reconfigure indicator.
setReconfQueueFlag(true);
@@ -285,7 +285,7 @@ TEST_F(D2ProcessTest, queueFullRecovery) {
// Start queue manager with known good config.
ASSERT_TRUE(runWithConfig(valid_d2_config));
- D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
+ const D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
// Set the maximum queue size to manageable number.
size_t max_queue_size = 5;
@@ -335,7 +335,7 @@ TEST_F(D2ProcessTest, queueFullRecovery) {
/// verifies that checkQueueStatus reacts properly to recover.
TEST_F(D2ProcessTest, queueErrorRecovery) {
ASSERT_TRUE(runWithConfig(valid_d2_config));
- D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
+ const D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
// Since we are not really receiving, we have to stage an error.
queue_mgr->stopListening(D2QueueMgr::STOPPED_RECV_ERROR);
@@ -474,7 +474,7 @@ TEST_F(D2ProcessTest, shutdownArgs) {
/// returning its result.
TEST_F(D2ProcessTest, canShutdown) {
ASSERT_TRUE(runWithConfig(valid_d2_config));
- D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
+ const D2QueueMgrPtr& queue_mgr = getD2QueueMgr();
// Shutdown flag is false. Method should return false for all types.
EXPECT_EQ(false, checkCanShutdown(SD_NORMAL));
@@ -539,7 +539,7 @@ TEST_F(D2ProcessTest, canShutdown) {
// Now use update manager to dequeue the request and make a transaction.
// This lets us verify transaction list not empty logic.
- D2UpdateMgrPtr& update_mgr = getD2UpdateMgr();
+ const D2UpdateMgrPtr& update_mgr = getD2UpdateMgr();
ASSERT_TRUE(update_mgr);
ASSERT_NO_THROW(update_mgr->sweep());
ASSERT_EQ(0, queue_mgr->getQueueSize());
diff --git a/src/lib/dhcp_ddns/ncr_io.h b/src/lib/dhcp_ddns/ncr_io.h
index 6691958..94d08f7 100644
--- a/src/lib/dhcp_ddns/ncr_io.h
+++ b/src/lib/dhcp_ddns/ncr_io.h
@@ -285,7 +285,7 @@ public:
return (listening_);
}
- /// @brief Returns true if the listener is has an IO call in progress.
+ /// @brief Returns true if the listener has an IO call in progress.
///
/// A true value indicates that the listener has an asynchronous IO in
/// progress which will complete at some point in the future. Completion
More information about the bind10-changes
mailing list