From b64a3b5d1ecc5e8145a7529f15455bcb28d06376 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Fri, 7 Dec 2012 17:06:53 +0100 Subject: [PATCH] Some work on DNS discovery - look up all seeds in parallel, pick 30 results, shuffle them and return. Allow users to specify a timeout. Return after the 30 results are found or we run out of time, whichever is first. Should smooth bumpy startup delays caused by occasional seed breakdowns. --- .../com/google/bitcoin/core/PeerGroup.java | 10 +-- .../bitcoin/discovery/DnsDiscovery.java | 90 +++++++++++-------- .../bitcoin/discovery/IrcDiscovery.java | 6 +- .../bitcoin/discovery/PeerDiscovery.java | 5 +- .../google/bitcoin/discovery/SeedPeers.java | 3 +- .../google/bitcoin/core/PeerGroupTest.java | 3 +- .../bitcoin/discovery/SeedPeersTest.java | 3 +- .../google/bitcoin/examples/PrintPeers.java | 5 +- 8 files changed, 75 insertions(+), 50 deletions(-) diff --git a/core/src/main/java/com/google/bitcoin/core/PeerGroup.java b/core/src/main/java/com/google/bitcoin/core/PeerGroup.java index cf520297f..bbdf84322 100644 --- a/core/src/main/java/com/google/bitcoin/core/PeerGroup.java +++ b/core/src/main/java/com/google/bitcoin/core/PeerGroup.java @@ -36,10 +36,7 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.util.*; -import java.util.concurrent.CopyOnWriteArraySet; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.ThreadFactory; +import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicInteger; import static com.google.common.base.Preconditions.checkArgument; @@ -413,10 +410,10 @@ public class PeerGroup { } protected void discoverPeers() throws PeerDiscoveryException { + long start = System.currentTimeMillis(); for (PeerDiscovery peerDiscovery : peerDiscoverers) { - // TODO: Run all peer discovery sources in parallel. InetSocketAddress[] addresses; - addresses = peerDiscovery.getPeers(); + addresses = peerDiscovery.getPeers(10, TimeUnit.SECONDS); synchronized (inactives) { for (int i = 0; i < addresses.length; i++) { inactives.add(new PeerAddress(addresses[i])); @@ -424,6 +421,7 @@ public class PeerGroup { if (inactives.size() > 0) break; } } + log.info("Peer discovery took {}msec", System.currentTimeMillis() - start); } /** Picks a peer from discovery and connects to it. If connection fails, picks another and tries again. */ diff --git a/core/src/main/java/com/google/bitcoin/discovery/DnsDiscovery.java b/core/src/main/java/com/google/bitcoin/discovery/DnsDiscovery.java index 01af469c4..fdb9395b6 100644 --- a/core/src/main/java/com/google/bitcoin/discovery/DnsDiscovery.java +++ b/core/src/main/java/com/google/bitcoin/discovery/DnsDiscovery.java @@ -17,22 +17,33 @@ package com.google.bitcoin.discovery; import com.google.bitcoin.core.NetworkParameters; +import com.google.common.collect.Sets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.net.InetAddress; import java.net.InetSocketAddress; -import java.util.HashSet; +import java.net.UnknownHostException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.Set; +import java.util.concurrent.*; /** - * Supports peer discovery through DNS.

+ *

Supports peer discovery through DNS.

* - * This class does not support the testnet as currently there are no DNS servers providing testnet hosts. - * If this class is being used for testnet you must specify the hostnames to use.

+ *

This class does not support the testnet as currently there are no DNS servers providing testnet hosts. + * If this class is being used for testnet you must specify the hostnames to use.

* - * Failure to resolve individual host names will not cause an Exception to be thrown. + *

Failure to resolve individual host names will not cause an Exception to be thrown. * However, if all hosts passed fail to resolve a PeerDiscoveryException will be thrown during getPeers(). + *

+ * + *

DNS seeds do not attempt to enumerate every peer on the network. {@link DnsDiscovery#getPeers(long, java.util.concurrent.TimeUnit)} + * will return up to 30 random peers from the set of those returned within the timeout period. If you want more peers + * to connect to, you need to discover them via other means (like addr broadcasts). + *

*/ public class DnsDiscovery implements PeerDiscovery { private static final Logger log = LoggerFactory.getLogger(DnsDiscovery.class); @@ -67,42 +78,51 @@ public class DnsDiscovery implements PeerDiscovery { this.netParams = netParams; } - public InetSocketAddress[] getPeers() throws PeerDiscoveryException { - Set addresses = new HashSet(); + public InetSocketAddress[] getPeers(long timeoutValue, TimeUnit timeoutUnit) throws PeerDiscoveryException { + final BlockingQueue queue = new LinkedBlockingQueue(); - /* - * Keep track of how many lookups failed vs. succeeded. - * We'll throw an exception only if all the lookups fail. - * We don't want to throw an exception if only one of many lookups fails. - */ - int failedLookups = 0; - - for (String hostName : hostNames) { - try { - InetAddress[] hostAddresses = InetAddress.getAllByName(hostName); - - for (InetAddress inetAddress : hostAddresses) { - // DNS isn't going to provide us with the port. - // Grab the port from the specified NetworkParameters. - InetSocketAddress socketAddress = new InetSocketAddress(inetAddress, netParams.port); - - // Only add the new address if it's not already in the combined list. - if (!addresses.contains(socketAddress)) { - addresses.add(socketAddress); + ArrayList seeds = new ArrayList(Arrays.asList(hostNames)); + // Java doesn't have an async DNS API so we have to do all lookups in a thread pool, as sometimes seeds go + // hard down and it takes ages to give up and move on. + ExecutorService pool = Executors.newFixedThreadPool(seeds.size()); + for (final String seed : seeds) { + pool.submit(new Runnable() { + public void run() { + try { + InetAddress[] addrs = InetAddress.getAllByName(seed); + for (InetAddress addr : addrs) queue.put(new InetSocketAddress(addr, netParams.port)); + } catch (UnknownHostException e) { + log.warn("Unable to resolve {}", seed); + } catch (InterruptedException e) { + // Silently go away. } } - } catch (Exception e) { - failedLookups++; - log.info("DNS lookup for " + hostName + " failed."); - - if (failedLookups == hostNames.length) { - // All the lookups failed. - // Throw the discovery exception and include the last inner exception. - throw new PeerDiscoveryException("DNS resolution for all hosts failed.", e); + }); + } + // The queue will fill up with resolutions. Let's wait until we got at least 30 or we run out of time. + final long timeout = timeoutUnit.toMillis(timeoutValue); + long start = System.currentTimeMillis(); + Set addrs = Sets.newHashSet(); + while (addrs.size() < 30) { + try { + long pollTime = timeout - (System.currentTimeMillis() - start); + if (pollTime < 0) break; + InetSocketAddress a = queue.poll(pollTime, TimeUnit.MILLISECONDS); + if (a == null) { + break; } + addrs.add(a); + } catch (InterruptedException e) { + break; } } - return addresses.toArray(new InetSocketAddress[]{}); + if (addrs.size() == 0) { + throw new PeerDiscoveryException("Unable to find any peers via DNS"); + } + ArrayList shuffledAddrs = new ArrayList(addrs); + Collections.shuffle(shuffledAddrs); + pool.shutdown(); + return shuffledAddrs.toArray(new InetSocketAddress[]{}); } /** diff --git a/core/src/main/java/com/google/bitcoin/discovery/IrcDiscovery.java b/core/src/main/java/com/google/bitcoin/discovery/IrcDiscovery.java index 102bbf1c4..dd5c8ab77 100644 --- a/core/src/main/java/com/google/bitcoin/discovery/IrcDiscovery.java +++ b/core/src/main/java/com/google/bitcoin/discovery/IrcDiscovery.java @@ -29,6 +29,7 @@ import java.net.Socket; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Random; +import java.util.concurrent.TimeUnit; /** * IrcDiscovery provides a way to find network peers by joining a pre-agreed rendevouz point on the LFnet IRC network. @@ -85,9 +86,10 @@ public class IrcDiscovery implements PeerDiscovery { } /** * Returns a list of peers that were found in the IRC channel. Note that just because a peer appears in the list - * does not mean it is accepting connections. + * does not mean it is accepting connections. BUG: the given timeout values are ignored. */ - public InetSocketAddress[] getPeers() throws PeerDiscoveryException { + public InetSocketAddress[] getPeers(long timeoutValue, TimeUnit timeoutUnit) throws PeerDiscoveryException { + // TODO: Make the timeout value be respected. ArrayList addresses = new ArrayList(); connection = null; try { diff --git a/core/src/main/java/com/google/bitcoin/discovery/PeerDiscovery.java b/core/src/main/java/com/google/bitcoin/discovery/PeerDiscovery.java index 87e7530ce..ee36b86a2 100644 --- a/core/src/main/java/com/google/bitcoin/discovery/PeerDiscovery.java +++ b/core/src/main/java/com/google/bitcoin/discovery/PeerDiscovery.java @@ -17,16 +17,17 @@ package com.google.bitcoin.discovery; import java.net.InetSocketAddress; +import java.util.concurrent.TimeUnit; /** - * A PeerDiscovery object is responsible for finding addresses of other nodes in the BitCoin P2P network. Note that + * A PeerDiscovery object is responsible for finding addresses of other nodes in the Bitcoin P2P network. Note that * the addresses returned may or may not be accepting connections. */ public interface PeerDiscovery { // TODO: Flesh out this interface a lot more. /** Returns an array of addresses. This method may block. */ - InetSocketAddress[] getPeers() throws PeerDiscoveryException; + InetSocketAddress[] getPeers(long timeoutValue, TimeUnit timeoutUnit) throws PeerDiscoveryException; /** Stops any discovery in progress when we want to shut down quickly. */ void shutdown(); diff --git a/core/src/main/java/com/google/bitcoin/discovery/SeedPeers.java b/core/src/main/java/com/google/bitcoin/discovery/SeedPeers.java index 7286b5981..0358ecf2f 100644 --- a/core/src/main/java/com/google/bitcoin/discovery/SeedPeers.java +++ b/core/src/main/java/com/google/bitcoin/discovery/SeedPeers.java @@ -20,6 +20,7 @@ import com.google.bitcoin.core.NetworkParameters; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; +import java.util.concurrent.TimeUnit; /** * SeedPeers stores a pre-determined list of Bitcoin node addresses. These nodes are selected based on being @@ -58,7 +59,7 @@ public class SeedPeers implements PeerDiscovery { /** * Returns an array containing all the Bitcoin nodes within the list. */ - public InetSocketAddress[] getPeers() throws PeerDiscoveryException { + public InetSocketAddress[] getPeers(long timeoutValue, TimeUnit timeoutUnit) throws PeerDiscoveryException { try { return allPeers(); } catch (UnknownHostException e) { diff --git a/core/src/test/java/com/google/bitcoin/core/PeerGroupTest.java b/core/src/test/java/com/google/bitcoin/core/PeerGroupTest.java index 5325fc6b6..d89084062 100644 --- a/core/src/test/java/com/google/bitcoin/core/PeerGroupTest.java +++ b/core/src/test/java/com/google/bitcoin/core/PeerGroupTest.java @@ -29,6 +29,7 @@ import java.util.Date; import java.util.HashSet; import java.util.Set; import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; import static org.junit.Assert.*; @@ -85,7 +86,7 @@ public class PeerGroupTest extends TestWithNetworkConnections { final boolean[] result = new boolean[1]; result[0] = false; peerGroup.addPeerDiscovery(new PeerDiscovery() { - public InetSocketAddress[] getPeers() throws PeerDiscoveryException { + public InetSocketAddress[] getPeers(long unused, TimeUnit unused2) throws PeerDiscoveryException { if (result[0] == false) { // Pretend we are not connected to the internet. result[0] = true; diff --git a/core/src/test/java/com/google/bitcoin/discovery/SeedPeersTest.java b/core/src/test/java/com/google/bitcoin/discovery/SeedPeersTest.java index 243b3785a..6b83914bc 100644 --- a/core/src/test/java/com/google/bitcoin/discovery/SeedPeersTest.java +++ b/core/src/test/java/com/google/bitcoin/discovery/SeedPeersTest.java @@ -19,6 +19,7 @@ import com.google.bitcoin.core.NetworkParameters; import org.junit.Test; import java.net.InetSocketAddress; +import java.util.concurrent.TimeUnit; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.notNullValue; @@ -43,7 +44,7 @@ public class SeedPeersTest { @Test public void getPeers_length() throws Exception{ SeedPeers seedPeers = new SeedPeers(NetworkParameters.prodNet()); - InetSocketAddress[] addresses = seedPeers.getPeers(); + InetSocketAddress[] addresses = seedPeers.getPeers(0, TimeUnit.SECONDS); assertThat(addresses.length, equalTo(SeedPeers.seedAddrs.length)); } } diff --git a/examples/src/main/java/com/google/bitcoin/examples/PrintPeers.java b/examples/src/main/java/com/google/bitcoin/examples/PrintPeers.java index 4de76ef32..0636e3f31 100644 --- a/examples/src/main/java/com/google/bitcoin/examples/PrintPeers.java +++ b/examples/src/main/java/com/google/bitcoin/examples/PrintPeers.java @@ -32,6 +32,7 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.TimeUnit; /** * Prints a list of IP addresses connected to the rendezvous point on the LFnet IRC channel. @@ -64,7 +65,7 @@ public class PrintPeers { System.out.println("-> " + message); } }; - ircPeers = d.getPeers(); + ircPeers = d.getPeers(10, TimeUnit.SECONDS); printPeers(ircPeers); printElapsed(start); } @@ -72,7 +73,7 @@ public class PrintPeers { private static void printDNS() throws PeerDiscoveryException { long start = System.currentTimeMillis(); DnsDiscovery dns = new DnsDiscovery(NetworkParameters.prodNet()); - dnsPeers = dns.getPeers(); + dnsPeers = dns.getPeers(10, TimeUnit.SECONDS); printPeers(dnsPeers); printElapsed(start); }