Skip to content

Commit

Permalink
[5649] Use WatchSocket to break sends in Command Manager.
Browse files Browse the repository at this point in the history
  • Loading branch information
msiodelski committed Jun 14, 2018
1 parent 9256e19 commit f7b8869
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 3 deletions.
45 changes: 42 additions & 3 deletions src/lib/config/command_mgr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <cc/json_feed.h>
#include <dhcp/iface_mgr.h>
#include <config/config_log.h>
#include <util/watch_socket.h>
#include <boost/bind.hpp>
#include <boost/enable_shared_from_this.hpp>
#include <array>
Expand Down Expand Up @@ -51,6 +52,11 @@ class Connection : public boost::enable_shared_from_this<Connection> {
/// Manager to cause the blocking call to @c select() to return as soon as
/// a transmission over the control socket is received.
///
/// It installs two external sockets on the @IfaceMgr to break synchronous
/// calls to @select(). The @c WatchSocket is used for send operations
/// over the connection. The native socket is used for signaling reads
/// over the connection.
///
/// @param io_service IOService object used to handle the asio operations
/// @param socket Pointer to the object representing a socket which is used
/// for data transmission.
Expand All @@ -63,14 +69,16 @@ class Connection : public boost::enable_shared_from_this<Connection> {
const unsigned short timeout)
: socket_(socket), timeout_timer_(*io_service), timeout_(timeout),
buf_(), response_(), connection_pool_(connection_pool), feed_(),
response_in_progress_(false) {
response_in_progress_(false), watch_socket_(new util::WatchSocket()) {

LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_CONNECTION_OPENED)
.arg(socket_->getNative());

// Callback value of 0 is used to indicate that callback function is
// not installed.
isc::dhcp::IfaceMgr::instance().addExternalSocket(watch_socket_->getSelectFd(), 0);
isc::dhcp::IfaceMgr::instance().addExternalSocket(socket_->getNative(), 0);

// Initialize state model for receiving and preparsing commands.
feed_.initModel();

Expand Down Expand Up @@ -102,7 +110,16 @@ class Connection : public boost::enable_shared_from_this<Connection> {
LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_CONNECTION_CLOSED)
.arg(socket_->getNative());

isc::dhcp::IfaceMgr::instance().deleteExternalSocket(watch_socket_->getSelectFd());
isc::dhcp::IfaceMgr::instance().deleteExternalSocket(socket_->getNative());

// Close watch socket and log errors if occur.
std::string watch_error;
if (!watch_socket_->closeSocket(watch_error)) {
LOG_ERROR(command_logger, COMMAND_WATCH_SOCKET_CLOSE_ERROR)
.arg(watch_error);
}

socket_->close();
timeout_timer_.cancel();
}
Expand All @@ -123,8 +140,6 @@ class Connection : public boost::enable_shared_from_this<Connection> {
socket_->asyncReceive(&buf_[0], sizeof(buf_),
boost::bind(&Connection::receiveHandler,
shared_from_this(), _1, _2));


}

/// @brief Starts asynchronous send over the unix domain socket.
Expand All @@ -138,6 +153,17 @@ class Connection : public boost::enable_shared_from_this<Connection> {
size_t chunk_size = (response_.size() < BUF_SIZE) ? response_.size() : BUF_SIZE;
socket_->asyncSend(&response_[0], chunk_size,
boost::bind(&Connection::sendHandler, shared_from_this(), _1, _2));

// Asynchronous send has been scheduled and we need to indicate this
// to break the synchronous select(). The handler should clear this
// status when invoked.
try {
watch_socket_->markReady();

} catch (const std::exception& ex) {
LOG_ERROR(command_logger, COMMAND_WATCH_SOCKET_MARK_READY_ERROR)
.arg(ex.what());
}
}

/// @brief Handler invoked when the data is received over the control
Expand Down Expand Up @@ -201,6 +227,9 @@ class Connection : public boost::enable_shared_from_this<Connection> {
/// result of server reconfiguration.
bool response_in_progress_;

/// @brief Pointer to watch socket instance used to signal that the socket
/// is ready for read or write.
util::WatchSocketPtr watch_socket_;
};

/// @brief Pointer to the @c Connection.
Expand Down Expand Up @@ -362,6 +391,16 @@ Connection::receiveHandler(const boost::system::error_code& ec,
void
Connection::sendHandler(const boost::system::error_code& ec,
size_t bytes_transferred) {
// Clear the watch socket so as the future send operation can mark it
// again to interrupt the synchronous select() call.
try {
watch_socket_->clearReady();

} catch (const std::exception& ex) {
LOG_ERROR(command_logger, COMMAND_WATCH_SOCKET_CLEAR_ERROR)
.arg(ex.what());
}

if (ec) {
// If an error occurred, log this error and stop the connection.
if (ec.value() != boost::asio::error::operation_aborted) {
Expand Down
18 changes: 18 additions & 0 deletions src/lib/config/config_messages.mes
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,21 @@ over command socket identifier by the specified file descriptor.
% COMMAND_SOCKET_WRITE_FAIL Error while writing to command socket %1 : %2
This error message indicates that an error was encountered while
attempting to send a response to the command socket.

% COMMAND_WATCH_SOCKET_CLEAR_ERROR watch socket failed to clear: %1
This error message is issued when the command manager was unable to reset
the ready status after completing a send. This is a programmatic error
that should be reported. The command manager may or may not continue
to operate correctly.

% COMMAND_WATCH_SOCKET_CLOSE_ERROR watch socket failed to close: %1
This error message is issued when command manager attempted to close the
socket used for indicating the ready status for send operations. This
should not have any negative impact on the operation of the command
manager as it happens when the connection is being terminated.

% COMMAND_WATCH_SOCKET_MARK_READY_ERROR watch socket failed to mark ready: %1
This error message is issued when the command manager was unable to set
ready status after scheduling asynchronous send. This is programmatic error
that should be reported. The command manager may or may not continue
to operate correctly.

0 comments on commit f7b8869

Please sign in to comment.