3
0
mirror of https://github.com/Qortal/altcoinj.git synced 2025-01-31 07:12:17 +00:00

Take only as much time as necessary to open circuits for peer-IP DNS lookups

This patch addresses two suboptimal behaviors in the `TorDiscovery` class:

1. Currently it uses the DNS-lookup timeout as the timeout for opening
   circuits as well as for actually looking up the IP numbers.  It may
   be this timeout value, which is five seconds hard-coded into the
   `PeerGroup` class, was not intended for this purpose.  Moreover,
   the `TorConfig` class has a value, circuitBuildTimeout, apparently
   for this purpose; at least its default of 60 seconds may be more
   appropriate for this purpose than the five-second lookup timeout.

2. Currently, the process of opening circuits for the DNS lookups will
   take exactly as long as the timeout value, no more, no less, which
   practically guarantees that the client will either wait longer than
   necessary, or else experience timeout errors that could have been
   avoided by waiting a bit longer.

The changes herein use the return value of`TorConfig.getCircuitBuildTimeout()`
as the timeout for opening circuits, rather than using the DNS lookup
timeout value that is passed by `PeerGroup` as an argument to
`PeerDiscovery.getPeers()`.  Moreover, as soon as a circuit either has
been opened or failed to be opened for each router, the DNS lookups
then begin without waiting for the timeout time to elapse.

Notes:

1. There are no tests with these changes.  There don't seem to be any
   tests for the `TorDiscovery` class.  There probably ought to be.

2. The `TorDiscovery` class's DNS lookup procedure has the same
   timeout characteristic as the one this patch removes from the
   circuit-opening procedure, namely it waits for as long as the
   timeout value, no more no less.  If the changes proposed in this
   patch are acceptable, I would consider making the same change for
   the process of looking up the peer IP numbers.
This commit is contained in:
Adam Mackler 2014-09-09 08:51:30 -04:00 committed by Mike Hearn
parent 691de1d22b
commit 34214533c9

View File

@ -16,10 +16,12 @@
package com.google.bitcoin.net.discovery;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.bitcoin.core.NetworkParameters;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
@ -45,6 +47,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
@ -64,7 +67,8 @@ import static java.util.Collections.singleton;
public class TorDiscovery implements PeerDiscovery {
private static final Logger log = LoggerFactory.getLogger(TorDiscovery.class);
public static final int MINIMUM_ROUTER_COUNT = 4;
public static final int MINIMUM_ROUTER_LOOKUP_COUNT = 10;
public static final int ROUTER_LOOKUP_COUNT = 10;
public static final int MINIMUM_ROUTER_LOOKUP_COUNT = 6;
public static final int RECEIVE_RETRIES = 3;
public static final int RESOLVE_STREAM_ID = 0x1000; // An arbitrary stream ID
public static final int RESOLVE_CNAME = 0x00;
@ -121,13 +125,14 @@ public class TorDiscovery implements PeerDiscovery {
ArrayList<ExitTarget> dummyTargets = Lists.newArrayList();
// Collect exit nodes until we have enough
while (routers.size() < MINIMUM_ROUTER_LOOKUP_COUNT) {
while (routers.size() < ROUTER_LOOKUP_COUNT) {
Router router = pathChooser.chooseExitNodeForTargets(dummyTargets);
routers.add(router);
}
try {
List<Circuit> circuits = getCircuits(timeoutValue, timeoutUnit, routers);
List<Circuit> circuits =
getCircuits(torClient.getConfig().getCircuitBuildTimeout(), TimeUnit.MILLISECONDS, routers);
if (circuits.isEmpty())
throw new PeerDiscoveryException("Failed to open any circuit within " +
String.valueOf(timeoutValue) + " " + timeoutUnit);
@ -146,34 +151,40 @@ public class TorDiscovery implements PeerDiscovery {
}
private List<Circuit> getCircuits(long timeoutValue, TimeUnit timeoutUnit, Set<Router> routers) throws InterruptedException {
checkArgument(routers.size() >= MINIMUM_ROUTER_LOOKUP_COUNT, "Set of {} routers is smaller than required minimum {}",
routers.size(), MINIMUM_ROUTER_LOOKUP_COUNT);
createThreadPool(routers.size());
try {
List<ListenableFuture<Circuit>> circuitFutures = Lists.newArrayList();
final CountDownLatch doneSignal = new CountDownLatch(MINIMUM_ROUTER_LOOKUP_COUNT);
for (final Router router : routers) {
circuitFutures.add(threadPool.submit(new Callable<Circuit>() {
ListenableFuture<Circuit> openCircuit = threadPool.submit(new Callable<Circuit>() {
@Override
public Circuit call() throws Exception {
return torClient.getCircuitManager().openInternalCircuitTo(Lists.newArrayList(router));
}
}));
});
Futures.addCallback(openCircuit, new FutureCallback<Circuit>() {
public void onSuccess(Circuit circuit) {
doneSignal.countDown();
}
public void onFailure(Throwable thrown) {
doneSignal.countDown();
}
});
circuitFutures.add(openCircuit);
}
int timeouts = 0;
threadPool.awaitTermination(timeoutValue, timeoutUnit);
for (ListenableFuture<Circuit> future : circuitFutures) {
if (!future.isDone()) {
timeouts++;
future.cancel(true);
}
}
if (timeouts > 0)
log.warn("{} DNS lookup circuits timed out", timeouts);
boolean countedDown = doneSignal.await(timeoutValue, timeoutUnit);
try {
List<Circuit> circuits = new ArrayList<Circuit>(Futures.successfulAsList(circuitFutures).get());
// Any failures will result in null entries. Remove them.
circuits.removeAll(singleton(null));
int failures = routers.size() - circuits.size();
if (failures > 0) log.warn("{} failures " + (countedDown ? "" : "(including timeout) ") +
"opening DNS lookup circuits", failures);
return circuits;
} catch (ExecutionException e) {
// Cannot happen, successfulAsList accepts failures
@ -209,7 +220,7 @@ public class TorDiscovery implements PeerDiscovery {
}
}
if (timeouts > 0)
log.warn("{} DNS lookup circuits timed out", timeouts);
log.warn("{} DNS lookups timed out", timeouts);
try {
List<Lookup> lookups = new ArrayList<Lookup>(Futures.successfulAsList(lookupFutures).get());