Skip to content

Commit

Permalink
Handle failures of individual dht routers
Browse files Browse the repository at this point in the history
If a single dht router is offline/unavailable, we should still be able
to successfully resolve the remainder and bootstrap.

Additionally, tweak how initialisation is handled. The list of initial
nodes and the DHT bootstrap nodes must be provided at the start. If both
are provided then the initial nodes will be used for bootstrapping prior
to attempting to resolve/use the DHT bootstrap nodes.

If an empty list of bootstrap nodes is provided, then no lookup will be
performed and only the 'initial nodes' will be used for bootstrapping.

If an empty list of initial nodes and an empty list of bootstrap nodes
are provided, then the engine will fail to initialise.
  • Loading branch information
alanmcgovern committed Sep 15, 2024
1 parent 24877e7 commit 9e98a44
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ static SocketAsyncEventArgs GetSocketAsyncEventArgs ()

static void HandleOperationCompleted (object? sender, SocketAsyncEventArgs e)
{
// Don't retain the TCS forever. Note we do not want to null out the byte[] buffer
// as we *do* want to retain that so that we can avoid the expensive SetBuffer calls.
var tcs = (ReusableTaskCompletionSource<int>) e.UserToken!;
SocketError error = e.SocketError;

Expand Down
37 changes: 21 additions & 16 deletions src/MonoTorrent.Dht/MonoTorrent.Dht.Tasks/InitialiseTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,6 @@ namespace MonoTorrent.Dht.Tasks
{
class InitialiseTask
{
static readonly string[] DefaultBootstrapRouters = new[] {
"router.bittorrent.com",
"router.utorrent.com",
"dht.transmissionbt.com"
};

// Choose a completely arbitrary value here. If we have at least this many
// nodes in the routing table we can consider it 'healthy' enough to allow
// the state to change to 'Ready' so torrents can begin searching for peers
Expand All @@ -57,7 +51,7 @@ class InitialiseTask
string[] BootstrapRouters { get; set; }

public InitialiseTask (DhtEngine engine)
: this (engine, Enumerable.Empty<Node> (), DefaultBootstrapRouters)
: this (engine, Enumerable.Empty<Node> (), DhtEngine.DefaultBootstrapRouters.ToArray ())
{

}
Expand All @@ -66,7 +60,7 @@ public InitialiseTask (DhtEngine engine, IEnumerable<Node> nodes, string[] boots
{
this.engine = engine;
initialNodes = nodes.ToList ();
BootstrapRouters = bootstrapRouters.Length == 0 ? DefaultBootstrapRouters : bootstrapRouters.ToArray ();
BootstrapRouters = bootstrapRouters;
initializationComplete = new TaskCompletionSource<object?> ();
}

Expand Down Expand Up @@ -100,13 +94,24 @@ async void BeginAsyncInit ()

async Task<Node[]> GenerateBootstrapNodes ()
{
var addresses = await Task.WhenAll (BootstrapRouters.Select (Dns.GetHostAddressesAsync));
return addresses
.SelectMany (t => t)
.Distinct ()
.Select (t => new IPEndPoint (t, 6881))
.Select (t => new Node (NodeId.Create (), t))
.ToArray ();
// Handle when one, or more, of the bootstrap nodes are offline
var results = new List<Node> ();

var tasks = BootstrapRouters.Select (Dns.GetHostAddressesAsync).ToList ();
while (tasks.Count > 0) {
var completed = await Task.WhenAny (tasks);
tasks.Remove (completed);

try {
var addresses = await completed;
foreach (var v in addresses)
results.Add (new Node (NodeId.Create (), new IPEndPoint (v, 6881)));
} catch {

}
}

return results.ToArray ();
}

async Task SendFindNode (IEnumerable<Node> newNodes)
Expand Down Expand Up @@ -139,7 +144,7 @@ async Task SendFindNode (IEnumerable<Node> newNodes)
}
}

if (initialNodes.Count > 0 && engine.RoutingTable.NeedsBootstrap)
if (initialNodes.Count > 0 && engine.RoutingTable.NeedsBootstrap && BootstrapRouters.Length > 0)
await new InitialiseTask (engine).ExecuteAsync ();
}
}
Expand Down
8 changes: 7 additions & 1 deletion src/MonoTorrent.Dht/MonoTorrent.Dht/DhtEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ class TransferMonitor : ITransferMonitor

public class DhtEngine : IDisposable, IDhtEngine
{
internal static readonly IList<string> DefaultBootstrapRouters = Array.AsReadOnly (new[] {
"router.bittorrent.com",
"router.utorrent.com",
"dht.transmissionbt.com"
});

static readonly TimeSpan DefaultAnnounceInternal = TimeSpan.FromMinutes (10);
static readonly TimeSpan DefaultMinimumAnnounceInterval = TimeSpan.FromMinutes (3);

Expand Down Expand Up @@ -272,7 +278,7 @@ public Task StartAsync ()
=> StartAsync (ReadOnlyMemory<byte>.Empty);

public Task StartAsync (ReadOnlyMemory<byte> initialNodes)
=> StartAsync (Node.FromCompactNode (BEncodedString.FromMemory (initialNodes)).Concat (PendingNodes), Array.Empty<string> ());
=> StartAsync (Node.FromCompactNode (BEncodedString.FromMemory (initialNodes)).Concat (PendingNodes), DefaultBootstrapRouters.ToArray ());

public Task StartAsync (params string[] bootstrapRouters)
=> StartAsync (Array.Empty<Node> (), bootstrapRouters);
Expand Down
4 changes: 1 addition & 3 deletions src/Tests/Tests.MonoTorrent.Dht/MonoTorrent.Dht/TaskTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,10 @@ public async Task Setup ()
[Repeat (10)]
public async Task InitialiseFailure ()
{
// FIXME: this flakily fails sometimes... why? DNS timeouts?
// Block until we've checked the status moved to Initializing
var errorSource = new TaskCompletionSource<object> ();
listener.MessageSent += (o, e) => errorSource.Task.GetAwaiter ().GetResult ();

await engine.StartAsync (new byte[26]);
await engine.StartAsync (new byte[26], Array.Empty<string> ());
Assert.AreEqual (DhtState.Initialising, engine.State);

// Then set an error and make sure the engine state moves to 'NotReady'
Expand Down

0 comments on commit 9e98a44

Please sign in to comment.