From 620868b5d6eeb0c0a757729daa16ca3ef734a86d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simon=20D=C3=A9saulniers?= <sim.desaulniers@gmail.com>
Date: Tue, 3 Jan 2017 17:48:12 -0500
Subject: [PATCH] routing_table: complete connecting state resolution

After a connectivity change, the DHT would enter the "connecting" state in order
to try to resolve good nodes. However, the previous algorithm would
probabilistically increase time for completion each time a node would fail to
resolve. We now avoid this by prioritizing non-expired nodes in the process.
Also, a subtle error would sensibly stop the process for 30 minutes due to using
the Node::NODE_EXPIRE_TIME constant instead of Node::MAX_RESPONSE_TIME.
---
 include/opendht/callbacks.h |  2 +-
 include/opendht/dhtrunner.h |  4 ++--
 src/dht.cpp                 | 15 +++++++--------
 src/dhtrunner.cpp           |  6 +++---
 src/routing_table.cpp       | 17 ++++++++++++++---
 5 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/include/opendht/callbacks.h b/include/opendht/callbacks.h
index cea81f1a..3a400725 100644
--- a/include/opendht/callbacks.h
+++ b/include/opendht/callbacks.h
@@ -46,7 +46,7 @@ struct OPENDHT_PUBLIC Config {
     /** DHT node ID */
     InfoHash node_id;
 
-    /** 
+    /**
      * DHT network ID. A node will only talk with other nodes having
      * the same network ID.
      * Network ID 0 (default) represents the main public network.
diff --git a/include/opendht/dhtrunner.h b/include/opendht/dhtrunner.h
index c7c20dc2..1a7eecfe 100644
--- a/include/opendht/dhtrunner.h
+++ b/include/opendht/dhtrunner.h
@@ -283,7 +283,7 @@ public:
     std::string getStorageLog(const InfoHash&) const;
     std::string getRoutingTablesLog(sa_family_t af) const;
     std::string getSearchesLog(sa_family_t af = AF_UNSPEC) const;
-    std::string getSearchLog(const InfoHash&, sa_family_t af = AF_UNSPEC) const;    
+    std::string getSearchLog(const InfoHash&, sa_family_t af = AF_UNSPEC) const;
     std::vector<SockAddr> getPublicAddress(sa_family_t af = AF_UNSPEC);
     std::vector<std::string> getPublicAddressStr(sa_family_t af = AF_UNSPEC);
 
@@ -370,7 +370,7 @@ private:
      * nodes so that the DHT node can recover quickly from losing connection
      * with the network.
      */
-    void tryBootstrapCoutinuously();
+    void tryBootstrapContinuously();
 
     void doRun(const sockaddr_in* sin4, const sockaddr_in6* sin6, SecureDhtConfig config);
     time_point loop_();
diff --git a/src/dht.cpp b/src/dht.cpp
index 28af1858..5c59a716 100644
--- a/src/dht.cpp
+++ b/src/dht.cpp
@@ -2829,11 +2829,12 @@ Dht::bucketMaintenance(RoutingTable& list)
     std::bernoulli_distribution rand_trial(1./8.);
     std::bernoulli_distribution rand_trial_38(1./38.);
 
+    bool sent {false};
     for (auto b = list.begin(); b != list.end(); ++b) {
         if (b->time < scheduler.time() - std::chrono::minutes(10) || b->nodes.empty()) {
             /* This bucket hasn't seen any positive confirmation for a long
-               time.  Pick a random id in this bucket's range, and send
-               a request to a random node. */
+               time. Pick a random id in this bucket's range, and send a request
+               to a random node. */
             InfoHash id = list.randomId(b);
             auto q = b;
             /* If the bucket is empty, we try to fill it from a neighbour.
@@ -2848,7 +2849,7 @@ Dht::bucketMaintenance(RoutingTable& list)
             }
 
             auto n = q->randomNode();
-            if (n) {
+            if (n and not n->isPendingMessage()) {
                 want_t want = -1;
 
                 if (network_engine.want() != want) {
@@ -2872,16 +2873,14 @@ Dht::bucketMaintenance(RoutingTable& list)
                         const auto& end = scheduler.time();
                         using namespace std::chrono;
                         DHT_LOG.d(n->id, "[node %s] bucket maintenance op expired after %llu ms", n->toString().c_str(), duration_cast<milliseconds>(end-start).count());
-                        scheduler.edit(nextNodesConfirmation, end + 3 * Node::NODE_EXPIRE_TIME);
+                        scheduler.edit(nextNodesConfirmation, end + Node::MAX_RESPONSE_TIME);
                     }
                 });
-                /* In order to avoid sending queries back-to-back,
-                   give up for now and reschedule us soon. */
-                return true;
+                sent = true;
             }
         }
     }
-    return false;
+    return sent;
 }
 
 void
diff --git a/src/dhtrunner.cpp b/src/dhtrunner.cpp
index 39557839..bd8a5b23 100644
--- a/src/dhtrunner.cpp
+++ b/src/dhtrunner.cpp
@@ -348,7 +348,7 @@ DhtRunner::loop_()
             // We have lost connection with the DHT.  Try to recover using bootstrap nodes.
             std::unique_lock<std::mutex> lck(bootstrap_mtx);
             bootstrap_nodes = bootstrap_nodes_all;
-            tryBootstrapCoutinuously();
+            tryBootstrapContinuously();
         } else {
             std::unique_lock<std::mutex> lck(bootstrap_mtx);
             bootstrap_nodes.clear();
@@ -618,7 +618,7 @@ DhtRunner::putEncrypted(const std::string& key, InfoHash to, Value&& value, Done
 }
 
 void
-DhtRunner::tryBootstrapCoutinuously()
+DhtRunner::tryBootstrapContinuously()
 {
     if (bootstrap_thread.joinable()) {
         if (bootstraping)
@@ -707,7 +707,7 @@ DhtRunner::bootstrap(const std::string& host, const std::string& service)
     std::lock_guard<std::mutex> lck(bootstrap_mtx);
     bootstrap_nodes_all.emplace_back(host, service);
     bootstrap_nodes.emplace_back(host, service);
-    tryBootstrapCoutinuously();
+    tryBootstrapContinuously();
 }
 
 void
diff --git a/src/routing_table.cpp b/src/routing_table.cpp
index cb6abe2c..576b3d14 100644
--- a/src/routing_table.cpp
+++ b/src/routing_table.cpp
@@ -17,10 +17,21 @@ Bucket::randomNode()
 {
     if (nodes.empty())
         return nullptr;
-    std::uniform_int_distribution<unsigned> rand_node(0, nodes.size()-1);
+    unsigned expired_node_count = std::count_if(nodes.cbegin(), nodes.cend(), [](const decltype(nodes)::value_type& node) {
+        return node->isExpired();
+    });
+    auto prioritize_not_expired = expired_node_count < nodes.size();
+
+    std::uniform_int_distribution<unsigned> rand_node(0, prioritize_not_expired
+            ? nodes.size() - expired_node_count - 1
+            : nodes.size()-1);
     unsigned nn = rand_node(rd);
-    for (auto& n : nodes)
-        if (not nn--) return n;
+    for (auto& n : nodes) {
+        if (not (prioritize_not_expired and n->isExpired())) {
+            if (not nn--)
+                return n;
+        }
+    }
     return nodes.back();
 }
 
-- 
GitLab