From c04ede490926c6d5bcecf7a4646bbbb04e3d428e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20D=C3=A9saulniers?= Date: Sat, 6 Aug 2016 21:13:35 -0400 Subject: [PATCH] dht: unify bootstrapSearch and refill method Since bootstrapSearch was rewritten to use the node cache, it made it clear that refill should be rewritten as such. However, then bootstrapSearch becomes irrelevant. An enhancement to refill is that it will let insertNode review its whole list of nodes and then let it insert nodes up to SEARCH_NODES nodes. The previous method would not garantee this behavior since, most of the times, it would pass less than SEARCH_NODES nodes to insertNode. --- include/opendht/dht.h | 10 ++++- include/opendht/network_engine.h | 6 +-- include/opendht/node_cache.h | 2 +- src/dht.cpp | 75 +++++++++----------------------- src/network_engine.cpp | 5 --- src/node_cache.cpp | 6 +-- 6 files changed, 37 insertions(+), 67 deletions(-) diff --git a/include/opendht/dht.h b/include/opendht/dht.h index 1c429c59..86485976 100644 --- a/include/opendht/dht.h +++ b/include/opendht/dht.h @@ -494,7 +494,15 @@ private: void announce(const InfoHash& id, sa_family_t af, std::shared_ptr value, DoneCallback callback, time_point created=time_point::max(), bool permanent = false); size_t listenTo(const InfoHash& id, sa_family_t af, GetCallback cb, Value::Filter f = Value::AllFilter(), const std::shared_ptr& q = {}); - void bootstrapSearch(Search& sr); + /** + * Refill the search with good nodes if possible. + * + * @param sr The search to refill. + * + * @return the number inserted nodes. + */ + unsigned refill(Search& sr); + Search *findSearch(unsigned short tid, sa_family_t af); void expireSearches(); diff --git a/include/opendht/network_engine.h b/include/opendht/network_engine.h index 7398fac6..65762201 100644 --- a/include/opendht/network_engine.h +++ b/include/opendht/network_engine.h @@ -362,14 +362,14 @@ public: void blacklistNode(const std::shared_ptr& n); - std::vector> getCachedNodes(const InfoHash& id, sa_family_t sa_f); + std::vector> getCachedNodes(const InfoHash& id, sa_family_t sa_f, size_t count) { + return cache.getCachedNodes(id, sa_f, count); + } private: /*************** * Constants * ***************/ - /* Total number of node needed from cache */ - static constexpr unsigned MAX_CACHE_NODE {14}; static constexpr long unsigned MAX_REQUESTS_PER_SEC {1600}; /* the length of a node info buffer in ipv4 format */ static const constexpr size_t NODE4_INFO_BUF_LEN {26}; diff --git a/include/opendht/node_cache.h b/include/opendht/node_cache.h index 4d63fd09..119f7d60 100644 --- a/include/opendht/node_cache.h +++ b/include/opendht/node_cache.h @@ -29,7 +29,7 @@ namespace dht { struct NodeCache { std::shared_ptr getNode(const InfoHash& id, sa_family_t family); std::shared_ptr getNode(const InfoHash& id, const sockaddr* sa, socklen_t sa_len, time_point now, bool confirmed); - std::vector> getCachedNodes(const InfoHash& id, sa_family_t sa_f, size_t total); + std::vector> getCachedNodes(const InfoHash& id, sa_family_t sa_f, size_t count); /** * Reset the connectivity state of every node, diff --git a/src/dht.cpp b/src/dht.cpp index 0ad74508..0297f0a4 100644 --- a/src/dht.cpp +++ b/src/dht.cpp @@ -329,7 +329,6 @@ struct Dht::Search { * @returns true if the node was not present and added to the search */ bool insertNode(const std::shared_ptr& n, time_point now, const Blob& token={}); - unsigned insertBucket(const Bucket&, time_point now); SearchNode* getNode(const std::shared_ptr& n) { auto srn = std::find_if(nodes.begin(), nodes.end(), [&](SearchNode& sn) { @@ -395,8 +394,6 @@ struct Dht::Search { bool removeExpiredNode(time_point now); - unsigned refill(const RoutingTable&, time_point now); - std::vector> getNodes() const; void clear() { @@ -844,13 +841,8 @@ Dht::searchStep(std::shared_ptr sr) sr->id.toString().c_str(), sr->af == AF_INET ? '4' : '6', sr->currentlySolicitedNodeCount()); sr->step_time = now; - if (sr->refill_time + Node::NODE_EXPIRE_TIME < now and sr->nodes.size()-sr->getNumberOfBadNodes() < SEARCH_NODES) { - if (auto added = sr->refill(sr->af == AF_INET ? buckets : buckets6, now)) { - sr->refill_time = now; - DHT_LOG.DEBUG("[search %s IPv%c] refilled with %u nodes", - sr->id.toString().c_str(), (sr->af == AF_INET) ? '4' : '6', added); - } - } + if (sr->refill_time + Node::NODE_EXPIRE_TIME < now and sr->nodes.size()-sr->getNumberOfBadNodes() < SEARCH_NODES) + refill(*sr); /* Check if the first TARGET_NODES (8) live nodes have replied. */ if (sr->isSynced(now)) { @@ -1028,18 +1020,6 @@ Dht::searchStep(std::shared_ptr sr) scheduler.edit(sr->nextSearchStep, sr->getNextStepTime(types, now)); } -/* Insert the contents of a bucket into a search structure. */ -unsigned -Dht::Search::insertBucket(const Bucket& b, time_point now) -{ - unsigned inserted = 0; - for (auto& n : b.nodes) { - if (not n->isExpired() and insertNode(n, now)) - inserted++; - } - return inserted; -} - bool Dht::Search::isSynced(time_point now) const { @@ -1230,43 +1210,30 @@ Dht::Search::getNextStepTime(const std::map& types, ti return next_step; } -void -Dht::bootstrapSearch(Dht::Search& sr) -{ - const auto& now = scheduler.time(); - const auto& list = network_engine.getCachedNodes(sr.id, sr.af); - if ( list.empty() ) { - DHT_LOG.ERR("No node found from cache"); - return; - } +unsigned Dht::refill(Dht::Search& sr) { + auto now = scheduler.time(); + /* we search for up to SEARCH_NODES good nodes. */ + auto cached_nodes = network_engine.getCachedNodes(sr.id, sr.af, SEARCH_NODES); - for ( auto& i : list) - sr.insertNode(i, now); - sr.refill_time = now; -} - -unsigned -Dht::Search::refill(const RoutingTable& r, time_point now) { - if (r.isEmpty() or r.front().af != af) + if (cached_nodes.empty()) { + DHT_LOG.ERR("[search %s IPv%c] no nodes from cache while refilling search", + sr.id.toString().c_str(), (sr.af == AF_INET) ? '4' : '6'); return 0; - unsigned added = 0; - auto num_bad_nodes = getNumberOfBadNodes(); - auto b = r.findBucket(id); - auto n = b; - while (nodes.size()-num_bad_nodes < SEARCH_NODES && (std::next(n) != r.end() || b != r.begin())) { - if (std::next(n) != r.end()) { - added += insertBucket(*std::next(n), now); - n = std::next(n); - } - if (b != r.begin()) { - added += insertBucket(*std::prev(b), now); - b = std::prev(b); - } } - return added; + unsigned inserted = 0; + for (auto& i : cached_nodes) { + /* try to insert the nodes. Search::insertNode will know how many to insert. */ + if (sr.insertNode(i, now)) + ++inserted; + } + DHT_LOG.DEBUG("[search %s IPv%c] refilled search with %u nodes from node cache", + sr.id.toString().c_str(), (sr.af == AF_INET) ? '4' : '6', inserted); + sr.refill_time = now; + return inserted; } + /* Start a search. */ std::shared_ptr Dht::search(const InfoHash& id, sa_family_t af, GetCallback gcb, QueryCallback qcb, DoneCallback dcb, Value::Filter f, Query q) @@ -1324,7 +1291,7 @@ Dht::search(const InfoHash& id, sa_family_t af, GetCallback gcb, QueryCallback q )); } - bootstrapSearch(*sr); + refill(*sr); if (sr->nextSearchStep) scheduler.edit(sr->nextSearchStep, sr->getNextStepTime(types, scheduler.time())); else diff --git a/src/network_engine.cpp b/src/network_engine.cpp index 760a88ed..674c78f6 100644 --- a/src/network_engine.cpp +++ b/src/network_engine.cpp @@ -275,11 +275,6 @@ NetworkEngine::isNodeBlacklisted(const sockaddr *sa, socklen_t salen) const return false; } -std::vector> -NetworkEngine::getCachedNodes(const InfoHash& id, sa_family_t sa_f) { - return cache.getCachedNodes(id, sa_f, MAX_CACHE_NODE); -} - void NetworkEngine::processMessage(const uint8_t *buf, size_t buflen, const sockaddr* from, socklen_t fromlen) { diff --git a/src/node_cache.cpp b/src/node_cache.cpp index a3d3a241..72655ab4 100644 --- a/src/node_cache.cpp +++ b/src/node_cache.cpp @@ -34,19 +34,19 @@ NodeCache::getNode(const InfoHash& id, const sockaddr* sa, socklen_t sa_len, tim } std::vector> -NodeCache::getCachedNodes(const InfoHash& id, sa_family_t sa_f, size_t total) { +NodeCache::getCachedNodes(const InfoHash& id, sa_family_t sa_f, size_t count) { const auto& c = (sa_f == AF_INET ? cache_4 : cache_6); auto it_p = c.lower_bound(id), it_n = it_p; std::vector> nodes; - nodes.reserve(std::min(c.size(), total)); + nodes.reserve(std::min(c.size(), count)); NodeMap::const_iterator it; if (it_p != c.begin()) /* Create 2 separate iterator if we could */ --it_p; - while (nodes.size() < total and (it_n != c.end() or it_p != c.end())) { + while (nodes.size() < count and (it_n != c.end() or it_p != c.end())) { /* If one of the iterator is at the end, then take the other one If they are both in middle of somewhere comapre both and take the closest to the id. */