Commit de3af2b8 authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[#691,!395] Addressed review comments 1

    Added TIMEOUT_DEFAULT_HTTP_CLIENT_REQUEST
    Removed virtual from callback declarations
    Added commentary to http/client.h
parent dbd6a288
......@@ -12,6 +12,7 @@
#include <ha_service_states.h>
#include <cc/command_interpreter.h>
#include <cc/data.h>
#include <config/timeouts.h>
#include <dhcp/iface_mgr.h>
#include <dhcpsrv/lease_mgr.h>
#include <dhcpsrv/lease_mgr_factory.h>
......@@ -854,7 +855,7 @@ HAService::asyncSendLeaseUpdate(const QueryPtrType& query,
runModel(HA_LEASE_UPDATES_COMPLETE_EVT);
}
},
HttpClient::RequestTimeout(10000),
HttpClient::RequestTimeout(TIMEOUT_DEFAULT_HTTP_CLIENT_REQUEST),
boost::bind(&HAService::clientConnectHandler, this, _1, _2),
boost::bind(&HAService::clientCloseHandler, this, _1)
);
......@@ -1057,7 +1058,7 @@ HAService::asyncSendHeartbeat() {
startHeartbeat();
runModel(HA_HEARTBEAT_COMPLETE_EVT);
},
HttpClient::RequestTimeout(10000),
HttpClient::RequestTimeout(TIMEOUT_DEFAULT_HTTP_CLIENT_REQUEST),
boost::bind(&HAService::clientConnectHandler, this, _1, _2),
boost::bind(&HAService::clientCloseHandler, this, _1)
);
......@@ -1147,7 +1148,7 @@ HAService::asyncDisableDHCPService(HttpClient& http_client,
error_message);
}
},
HttpClient::RequestTimeout(10000),
HttpClient::RequestTimeout(TIMEOUT_DEFAULT_HTTP_CLIENT_REQUEST),
boost::bind(&HAService::clientConnectHandler, this, _1, _2),
boost::bind(&HAService::clientCloseHandler, this, _1)
);
......@@ -1218,7 +1219,7 @@ HAService::asyncEnableDHCPService(HttpClient& http_client,
error_message);
}
},
HttpClient::RequestTimeout(10000),
HttpClient::RequestTimeout(TIMEOUT_DEFAULT_HTTP_CLIENT_REQUEST),
boost::bind(&HAService::clientConnectHandler, this, _1, _2),
boost::bind(&HAService::clientCloseHandler, this, _1)
);
......@@ -1621,12 +1622,10 @@ HAService::verifyAsyncResponse(const HttpResponsePtr& response) {
bool
HAService::clientConnectHandler(const boost::system::error_code& ec, int tcp_native_fd) {
if (!ec || ec.value() == boost::asio::error::in_progress) {
IfaceMgr::instance().addExternalSocket(tcp_native_fd,
[this]() {
// Callback is a NOP. Ready events handlers are run by an explicit
// call IOService ready in kea-dhcp<n> code. We are registering
// the socket to interrupt main-thread select().
});
// External socket callback is a NOP. Ready events handlers are run by an
// explicit call IOService ready in kea-dhcp<n> code. We are registering
// the socket only to interrupt main-thread select().
IfaceMgr::instance().addExternalSocket(tcp_native_fd, 0);
}
// If ec.value() == boost::asio::error::already_connected, we should already
......
......@@ -736,7 +736,7 @@ protected:
/// @param tcp_native_fd socket descriptor to register
/// @param returns true. Registeration cannot fail, and if ec indicates a real
/// error we want Connection logic to process it.
virtual bool clientConnectHandler(const boost::system::error_code& ec, int tcp_native_fd);
bool clientConnectHandler(const boost::system::error_code& ec, int tcp_native_fd);
/// @brief HttpClient close callback handler
///
......@@ -745,7 +745,7 @@ protected:
/// main-thread select()).
///
/// @param tcp_native_fd socket descriptor to register
virtual void clientCloseHandler(int tcp_native_fd);
void clientCloseHandler(int tcp_native_fd);
/// @brief Pointer to the IO service object shared between this hooks
/// library and the DHCP server.
......
......@@ -30,6 +30,14 @@ constexpr long TIMEOUT_AGENT_IDLE_CONNECTION_TIMEOUT = 30000;
/// to generate large responses, e.g. dump whole lease databse.
constexpr long TIMEOUT_AGENT_FORWARD_COMMAND = 60000;
/// @brief Timeout for the HTTP clients awaiting a response to a request.
///
/// This value is high to ensure that the client waits long enough
/// for the fulfilling server to generate a response. Specified
/// milliseconds.
constexpr long TIMEOUT_DEFAULT_HTTP_CLIENT_REQUEST = 10000;
} // end of namespace isc::config
} // end of namespace isc
......
......@@ -351,7 +351,7 @@ IfaceMgr::purgeBadSockets() {
std::vector<int> bad_fds;
BOOST_FOREACH(SocketCallbackInfo s, callbacks_) {
errno = 0;
if (fcntl(s.socket_, F_GETFD) < 0 && errno == EBADF) {
if (fcntl(s.socket_, F_GETFD) < 0 && (errno == EBADF)) {
bad_fds.push_back(s.socket_);
}
}
......
......@@ -59,6 +59,12 @@ class HttpClientImpl;
/// a request by trying to read from the socket (with message peeking). If
/// the socket is usable the client uses it to transmit the request.
///
/// This classes exposes the underlying TCP socket's descriptor for each
/// connection via connect and close callbacks. This is done to permit the
/// sockets to be monitored for IO readiness by external code that something
/// other than boost::asio (e.g.select() or epoll()), and would thus otherwise
/// starve the client's IOService and cause a backlog of ready event handlers.
///
/// All errors are reported to the caller via the callback function supplied
/// to the @ref HttpClient::asyncSendRequest. The IO errors are communicated
/// via the @c boost::system::error code value. The response parsing errors
......@@ -86,9 +92,9 @@ public:
///
/// Returned boolean value indicates whether the client should continue
/// connecting to the server (if true) or not (false).
/// It is passed the IO error code along with the native socket handle of
/// the connection's TCP socket. This always the socket's event readiness
/// to be monitored via select() or epoll.
/// It is passed the IO error code along with the native socket handle of
/// the connection's TCP socket. The passed socket descriptor may be used
/// to monitor the readiness of the events via select() or epoll().
///
/// @note Beware that the IO error code can be set to "in progress"
/// so a not null error code does not always mean the connect failed.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment