BIND 10 trac3329, updated. c03cc8c38810d3316faf205d8dda1d9bbbaab795 [3329] D2ClientMgr::sendRequest now calls client's error handler
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Feb 14 14:02:13 UTC 2014
The branch, trac3329 has been updated
via c03cc8c38810d3316faf205d8dda1d9bbbaab795 (commit)
from 72d32778775dc5517d208e300710f6a96e1848ee (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 c03cc8c38810d3316faf205d8dda1d9bbbaab795
Author: Thomas Markwalder <tmark at isc.org>
Date: Fri Feb 14 08:59:53 2014 -0500
[3329] D2ClientMgr::sendRequest now calls client's error handler
Altered D2ClientMgr::sendRequest to call the client's error handler if
in send mode and sendRequest fails. Prior to this it simply allowed any
sender exceptions to propagate.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcpsrv/d2_client_mgr.cc | 60 ++++++++++++++++++++----------
src/lib/dhcpsrv/d2_client_mgr.h | 27 ++++++++++++--
src/lib/dhcpsrv/dhcpsrv_messages.mes | 6 +++
src/lib/dhcpsrv/tests/d2_udp_unittest.cc | 20 +++++++++-
4 files changed, 88 insertions(+), 25 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcpsrv/d2_client_mgr.cc b/src/lib/dhcpsrv/d2_client_mgr.cc
index 111f370..c806bc4 100644
--- a/src/lib/dhcpsrv/d2_client_mgr.cc
+++ b/src/lib/dhcpsrv/d2_client_mgr.cc
@@ -264,7 +264,7 @@ D2ClientMgr::stopSender() {
}
// If its not null, call stop.
- if (name_change_sender_) {
+ if (amSending()) {
name_change_sender_->stopSending();
LOG_INFO(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_SENDER_STOPPED);
}
@@ -272,11 +272,36 @@ D2ClientMgr::stopSender() {
void
D2ClientMgr::sendRequest(dhcp_ddns::NameChangeRequestPtr& ncr) {
- if (!name_change_sender_) {
- isc_throw(D2ClientError, "D2ClientMgr::sendRequest sender is null");
+ if (!amSending()) {
+ // This is programmatic error so bust them for it.
+ isc_throw(D2ClientError, "D2ClientMgr::sendRequest not in send mode");
}
- name_change_sender_->sendRequest(ncr);
+ try {
+ name_change_sender_->sendRequest(ncr);
+ } catch (const std::exception& ex) {
+ LOG_ERROR(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_NCR_REJECTED)
+ .arg(ex.what()).arg((ncr ? ncr->toText() : " NULL "));
+ invokeClientErrorHandler(dhcp_ddns::NameChangeSender::ERROR, ncr);
+ }
+}
+
+void
+D2ClientMgr::invokeClientErrorHandler(const dhcp_ddns::NameChangeSender::
+ Result result,
+ dhcp_ddns::NameChangeRequestPtr& ncr) {
+ // Handler is mandatory to enter send mode but test it just to be safe.
+ if (!client_error_handler_) {
+ LOG_ERROR(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_HANDLER_NULL);
+ } else {
+ // Handler is not supposed to throw, but catch just in case.
+ try {
+ (client_error_handler_)(result, ncr);
+ } catch (const std::exception& ex) {
+ LOG_ERROR(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_ERROR_EXCEPTION)
+ .arg(ex.what());
+ }
+ }
}
size_t
@@ -288,6 +313,16 @@ D2ClientMgr::getQueueSize() const {
return(name_change_sender_->getQueueSize());
}
+size_t
+D2ClientMgr::getQueueMaxSize() const {
+ if (!name_change_sender_) {
+ isc_throw(D2ClientError, "D2ClientMgr::getQueueMaxSize sender is null");
+ }
+
+ return(name_change_sender_->getQueueMaxSize());
+}
+
+
const dhcp_ddns::NameChangeRequestPtr&
D2ClientMgr::peekAt(const size_t index) const {
@@ -314,21 +349,8 @@ D2ClientMgr::operator()(const dhcp_ddns::NameChangeSender::Result result,
LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
DHCPSRV_DHCP_DDNS_NCR_SENT).arg(ncr->toText());
} else {
- // Handler is mandatory but test it just to be safe.
- /// @todo Until we have a better feel for how errors need to be
- /// handled we farm it out to the application layer.
- if (client_error_handler_) {
- // Handler is not supposed to throw, but catch just in case.
- try {
- (client_error_handler_)(result, ncr);
- } catch (const std::exception& ex) {
- LOG_ERROR(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_ERROR_EXCEPTION)
- .arg(ex.what());
- }
- } else {
- LOG_ERROR(dhcpsrv_logger, DHCPSRV_DHCP_DDNS_HANDLER_NULL);
- }
- }
+ invokeClientErrorHandler(result, ncr);
+ }
}
int
diff --git a/src/lib/dhcpsrv/d2_client_mgr.h b/src/lib/dhcpsrv/d2_client_mgr.h
index c367551..be62c38 100644
--- a/src/lib/dhcpsrv/d2_client_mgr.h
+++ b/src/lib/dhcpsrv/d2_client_mgr.h
@@ -42,7 +42,7 @@ namespace dhcp {
/// @param result Result code of the send operation.
/// @param ncr NameChangeRequest which failed to send.
///
-/// @note Handlers are expected not to throw. In the event a hanlder does
+/// @note Handlers are expected not to throw. In the event a handler does
/// throw invoking code logs the exception and then swallows it.
typedef
boost::function<void(const dhcp_ddns::NameChangeSender::Result result,
@@ -248,17 +248,36 @@ public:
/// @brief Send the given NameChangeRequests to b10-dhcp-ddns
///
/// Passes NameChangeRequests to the NCR sender for transmission to
- /// b10-dhcp-ddns.
+ /// b10-dhcp-ddns. If the sender rejects the message, the client's error
+ /// handler will be invoked. The most likely cause for rejection is
+ /// the senders' queue has reached maximum capacity.
///
/// @param ncr NameChangeRequest to send
///
- /// @throw D2ClientError if sender instance is null. Underlying layer
- /// may throw NCRSenderExceptions exceptions.
+ /// @throw D2ClientError if sender instance is null or not in send
+ /// mode. Either of these represents a programmatic error.
void sendRequest(dhcp_ddns::NameChangeRequestPtr& ncr);
+ /// @brief Calls the client's error handler.
+ ///
+ /// Calls the error handler method set by startSender() when an
+ /// error occurs attempting to send a method. If the error handler
+ /// throws an exception it will be caught and logged.
+ ///
+ /// @param result contains that send outcome status.
+ /// @param ncr is a pointer to the NameChangeRequest that was attempted.
+ ///
+ /// This method is exception safe.
+ void invokeClientErrorHandler(const dhcp_ddns::NameChangeSender::
+ Result result,
+ dhcp_ddns::NameChangeRequestPtr& ncr);
+
/// @brief Returns the number of NCRs queued for transmission.
size_t getQueueSize() const;
+ /// @brief Returns the maximum number of NCRs allowed in the queue.
+ size_t getQueueMaxSize() const;
+
/// @brief Returns the nth NCR queued for transmission.
///
/// Note that the entry is not removed from the queue.
diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes
index 63f042a..5151851 100644
--- a/src/lib/dhcpsrv/dhcpsrv_messages.mes
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes
@@ -375,3 +375,9 @@ off. This should only occur if IO errors communicating with b10-dhcp-ddns
have been experienced. Any such errors should have preceding entries in the
log with details. No further attempts to communicate with b10-dhcp-ddns will
be made without intervention.
+
+% DHCPSRV_DHCP_DDNS_NCR_REJECTED NameChangeRequest rejected by the sender: %1, ncr: %2
+This is an error message indicating that NameChangeSender used to deliver DDNS
+update requests to b10-dhcp-ddns rejected the request. This most likely cause
+is the sender's queue has reached maximum capacity. This would imply that
+requests are being generated faster than they can be delivered.
diff --git a/src/lib/dhcpsrv/tests/d2_udp_unittest.cc b/src/lib/dhcpsrv/tests/d2_udp_unittest.cc
index 9953842..fb178a4 100644
--- a/src/lib/dhcpsrv/tests/d2_udp_unittest.cc
+++ b/src/lib/dhcpsrv/tests/d2_udp_unittest.cc
@@ -232,7 +232,7 @@ TEST_F(D2ClientMgrTest, udpSenderQueing) {
// Trying to send a NCR when not in send mode should fail.
dhcp_ddns::NameChangeRequestPtr ncr = buildTestNcr();
- EXPECT_THROW(sendRequest(ncr), dhcp_ddns::NcrSenderError);
+ EXPECT_THROW(sendRequest(ncr), D2ClientError);
// Place sender in send mode.
ASSERT_NO_THROW(startSender(getErrorHandler()));
@@ -380,6 +380,7 @@ TEST_F(D2ClientMgrTest, udpSendErrorHandlerThrow) {
ASSERT_EQ(0, error_handler_count_);
}
+/// @brief Tests that D2ClientMgr registers and unregisters with IfaceMgr.
TEST_F(D2ClientMgrTest, ifaceRegister) {
// Enable DDNS with server at 127.0.0.1/prot 53001 via UDP.
enableDdns("127.0.0.1", 530001, dhcp_ddns::NCR_UDP);
@@ -414,7 +415,7 @@ TEST_F(D2ClientMgrTest, ifaceRegister) {
ASSERT_EQ(2, callback_count_);
ASSERT_EQ(0, error_handler_count_);
- // Calling recevie again should have no affect.
+ // Calling receive again should have no affect.
IfaceMgr::instance().receive4(0, 0);
EXPECT_EQ(1, getQueueSize());
ASSERT_EQ(2, callback_count_);
@@ -453,4 +454,19 @@ TEST_F(D2ClientMgrTest, udpSuspendUpdates) {
ASSERT_EQ(1, getQueueSize());
}
+/// @brief Tests that invokeErrorHandler does not fail if there is no handler.
+TEST_F(D2ClientMgrTest, missingErrorHandler) {
+ // Ensure we aren't in send mode.
+ ASSERT_FALSE(ddnsEnabled());
+ ASSERT_FALSE(amSending());
+
+ // There is no error handler at this point, so invoking should not throw.
+ dhcp_ddns::NameChangeRequestPtr ncr;
+ ASSERT_NO_THROW(invokeClientErrorHandler(dhcp_ddns::NameChangeSender::ERROR,
+ ncr));
+
+ // Verify we didn't invoke the error handler, error count is zero.
+ ASSERT_EQ(0, error_handler_count_);
+}
+
} // end of anonymous namespace
More information about the bind10-changes
mailing list