Skip to content

Commit

Permalink
fix: schedule initial search use separate event from the generic work…
Browse files Browse the repository at this point in the history
… queue

By using `tcp_loop.dispatch` to schedule the initial search for a
channel, we are placing the callback into the same work queue that is
used by e.g. `MonitorBuilder::exec` to schedule the call to
`Channel::build`.  In situations where lots of channels are being
created simultaneously this can result in lots of single channel search
requests being sent because the work queue alternates between calls to
build a channel and the initial search.

In this commit we instead use a dedicated `evevent` to schedule the
initial search to allow the `initialSearchBucket` to be filled before we
send the initial search request. We delay the initial search by 10 ms to
give more time for the bucket to be filled.  See
github.com//pull/39 for a discussion of how this delay
was chosen.
  • Loading branch information
thomasives committed Apr 11, 2023
1 parent 64c480d commit d25a263
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
17 changes: 14 additions & 3 deletions src/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ namespace pvxs {
namespace client {

constexpr timeval bucketInterval{1,0};
constexpr timeval initialSearchDelay{0, 10000}; // 10 ms
constexpr size_t nBuckets = 30u;

// try not to fragment with usual MTU==1500
Expand Down Expand Up @@ -498,6 +499,8 @@ ContextImpl::ContextImpl(const Config& conf, const evbase& tcp_loop)
event_new(tcp_loop.base, searchTx6.sock, EV_READ|EV_PERSIST, &ContextImpl::onSearchS, this))
,searchTimer(__FILE__, __LINE__,
event_new(tcp_loop.base, -1, EV_TIMEOUT, &ContextImpl::tickSearchS, this))
,initialSearcher(__FILE__, __LINE__,
event_new(tcp_loop.base, -1, EV_TIMEOUT, &ContextImpl::initialSearchS, this))
,manager(UDPManager::instance(effective.shareUDP()))
,beaconCleaner(__FILE__, __LINE__,
event_new(manager.loop().base, -1, EV_TIMEOUT|EV_PERSIST, &ContextImpl::tickBeaconCleanS, this))
Expand Down Expand Up @@ -701,9 +704,8 @@ void ContextImpl::scheduleInitialSearch()
log_debug_printf(setup, "scheduleInitialSearch()%s\n", "");

initialSearchScheduled = true;
tcp_loop.dispatch([this]() {
tickSearch(SearchKind::initial);
});
if (event_add(initialSearcher.get(), &initialSearchDelay))
throw std::runtime_error("Unable to schedule initialSearcher");
}
}

Expand Down Expand Up @@ -1186,6 +1188,15 @@ void ContextImpl::tickSearchS(evutil_socket_t fd, short evt, void *raw)
}
}

void ContextImpl::initialSearchS(evutil_socket_t fd, short evt, void *raw)
{
try {
static_cast<ContextImpl*>(raw)->tickSearch(SearchKind::initial);
}catch(std::exception& e){
log_exc_printf(io, "Unhandled error in initial search callback: %s\n", e.what());
}
}

void ContextImpl::tickBeaconClean()
{
epicsTimeStamp now;
Expand Down
2 changes: 2 additions & 0 deletions src/clientimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ struct ContextImpl : public std::enable_shared_from_this<ContextImpl>
evbase tcp_loop;
const evevent searchRx4, searchRx6;
const evevent searchTimer;
const evevent initialSearcher;

// beacon handling done on UDP worker.
// we keep a ref here as long as beaconCleaner is in use
Expand Down Expand Up @@ -344,6 +345,7 @@ struct ContextImpl : public std::enable_shared_from_this<ContextImpl>
enum class SearchKind { discover, initial, check };
void tickSearch(SearchKind kind);
static void tickSearchS(evutil_socket_t fd, short evt, void *raw);
static void initialSearchS(evutil_socket_t fd, short evt, void *raw);
void tickBeaconClean();
static void tickBeaconCleanS(evutil_socket_t fd, short evt, void *raw);
void cacheClean(const std::string &name, Context::cacheAction force);
Expand Down

0 comments on commit d25a263

Please sign in to comment.