Skip to content

Commit

Permalink
[5649] IfaceMgr's select reacts on sending data over control channel.
Browse files Browse the repository at this point in the history
  • Loading branch information
msiodelski committed Jun 14, 2018
1 parent 9664641 commit 83f252d
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 4 deletions.
14 changes: 13 additions & 1 deletion src/hooks/dhcp/high_availability/ha_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ using namespace isc::http;
using namespace isc::log;
using namespace isc::util;

namespace {

/// @brief Timeout for synchronization of leases with partner.
///
/// This timeout is very high because in some cases the number of
/// gathered leases is huge. Syncing should not really take that
/// long, but if the partner doesn't respond we can't do anything
/// useful anyway. So, it doesn't really matter.
const long HA_SYNC_TIMEOUT = 60000;

}

namespace isc {
namespace ha {

Expand Down Expand Up @@ -1215,7 +1227,7 @@ HAService::asyncSyncLeases(http::HttpClient& http_client,
post_sync_action(error_message.empty(),
error_message);
}
});
}, HttpClient::RequestTimeout(HA_SYNC_TIMEOUT));
}

ConstElementPtr
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/dhcp/high_availability/ha_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ class HAService : public boost::noncopyable, public util::StateModel {
///
/// If there is an error while inserting or updating any of the leases
/// a warning message is logged and the process continues for the
/// remaining leases.
/// remaining leases. The timeout for synchronization is set to 1 minute.
///
/// @param http_client reference to the client to be used to communicate
/// with the other server.
Expand Down
30 changes: 28 additions & 2 deletions src/lib/dhcp/iface_mgr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -896,9 +896,11 @@ Pkt4Ptr IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
boost::scoped_ptr<SocketInfo> candidate;
IfacePtr iface;
fd_set sockets;
fd_set write_sockets;
int maxfd = 0;

FD_ZERO(&sockets);
FD_ZERO(&write_sockets);

/// @todo: marginal performance optimization. We could create the set once
/// and then use its copy for select(). Please note that select() modifies
Expand All @@ -922,6 +924,17 @@ Pkt4Ptr IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
if (!callbacks_.empty()) {
BOOST_FOREACH(SocketCallbackInfo s, callbacks_) {
FD_SET(s.socket_, &sockets);

// Sometimes we want to react to socket writes, not only to reads.
// However, current use cases are limited to CommandMgr which
// doesn't trigger any callbacks. For all use cases that install
// callbacks we don't react to socket reads being afraid what we
// can break.
/// @todo This whole solution is temporary and implemented for
/// #5649.
if (!s.callback_) {
FD_SET(s.socket_, &write_sockets);
}
if (maxfd < s.socket_) {
maxfd = s.socket_;
}
Expand All @@ -935,7 +948,7 @@ Pkt4Ptr IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
// zero out the errno to be safe
errno = 0;

int result = select(maxfd + 1, &sockets, NULL, NULL, &select_timeout);
int result = select(maxfd + 1, &sockets, &write_sockets, NULL, &select_timeout);

if (result == 0) {
// nothing received and timeout has been reached
Expand Down Expand Up @@ -1005,9 +1018,11 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */

boost::scoped_ptr<SocketInfo> candidate;
fd_set sockets;
fd_set write_sockets;
int maxfd = 0;

FD_ZERO(&sockets);
FD_ZERO(&write_sockets);

/// @todo: marginal performance optimization. We could create the set once
/// and then use its copy for select(). Please note that select() modifies
Expand All @@ -1032,6 +1047,17 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
BOOST_FOREACH(SocketCallbackInfo s, callbacks_) {
// Add it to the set as well
FD_SET(s.socket_, &sockets);

// Sometimes we want to react to socket writes, not only to reads.
// However, current use cases are limited to CommandMgr which
// doesn't trigger any callbacks. For all use cases that install
// callbacks we don't react to socket reads being afraid what we
// can break.
/// @todo This whole solution is temporary and implemented for
/// #5649.
if (!s.callback_) {
FD_SET(s.socket_, &write_sockets);
}
if (maxfd < s.socket_) {
maxfd = s.socket_;
}
Expand All @@ -1045,7 +1071,7 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
// zero out the errno to be safe
errno = 0;

int result = select(maxfd + 1, &sockets, NULL, NULL, &select_timeout);
int result = select(maxfd + 1, &sockets, &write_sockets, NULL, &select_timeout);

if (result == 0) {
// nothing received and timeout has been reached
Expand Down

0 comments on commit 83f252d

Please sign in to comment.