PeerGroup: bugfixes to backoff.

1) Do the wait even on the exception path so if discoverPeers throws, we don't bypass the sleep.
2) Move some field accesses inside the lock.

Resolves issue 527.
This commit is contained in:
Mike Hearn
2014-04-01 16:47:51 +02:00
committed by Andreas Schildbach
parent f399dafefc
commit 86e63fb048

View File

@@ -342,7 +342,7 @@ public class PeerGroup extends AbstractExecutionThreadService implements Transac
do { do {
try { try {
connectToAnyPeer(); connectToAnyPeer();
} catch(PeerDiscoveryException e) { } catch (PeerDiscoveryException e) {
groupBackoff.trackFailure(); groupBackoff.trackFailure();
} }
} while (isRunning() && countConnectedAndPendingPeers() < getMaxConnections()); } while (isRunning() && countConnectedAndPendingPeers() < getMaxConnections());
@@ -575,6 +575,8 @@ public class PeerGroup extends AbstractExecutionThreadService implements Transac
} }
protected void discoverPeers() throws PeerDiscoveryException { protected void discoverPeers() throws PeerDiscoveryException {
if (peerDiscoverers.isEmpty())
throw new PeerDiscoveryException("No peer discoverers registered");
long start = System.currentTimeMillis(); long start = System.currentTimeMillis();
Set<PeerAddress> addressSet = Sets.newHashSet(); Set<PeerAddress> addressSet = Sets.newHashSet();
for (PeerDiscovery peerDiscovery : peerDiscoverers) { for (PeerDiscovery peerDiscovery : peerDiscoverers) {
@@ -630,10 +632,10 @@ public class PeerGroup extends AbstractExecutionThreadService implements Transac
final State state = state(); final State state = state();
if (!(state == State.STARTING || state == State.RUNNING)) return; if (!(state == State.STARTING || state == State.RUNNING)) return;
final PeerAddress addr; PeerAddress addr = null;
long nowMillis = Utils.currentTimeMillis(); long nowMillis = Utils.currentTimeMillis();
long retryTime = 0;
lock.lock(); lock.lock();
try { try {
if (!haveReadyInactivePeer(nowMillis)) { if (!haveReadyInactivePeer(nowMillis)) {
@@ -646,18 +648,21 @@ public class PeerGroup extends AbstractExecutionThreadService implements Transac
return; return;
} }
addr = inactives.poll(); addr = inactives.poll();
retryTime = backoffMap.get(addr).getRetryTime();
} finally { } finally {
// discoverPeers might throw an exception if something goes wrong: we then hit this path with addr == null.
retryTime = Math.max(retryTime, groupBackoff.getRetryTime());
lock.unlock(); lock.unlock();
} if (retryTime > nowMillis) {
// Sleep until retry time
// Delay if any backoff is required final long millis = retryTime - nowMillis;
long retryTime = Math.max(backoffMap.get(addr).getRetryTime(), groupBackoff.getRetryTime()); log.info("Waiting {} msec before next connect attempt", millis, addr == null ? "" : " to " + addr);
if (retryTime > nowMillis) { Utils.sleep(millis);
// Sleep until retry time }
Utils.sleep(retryTime - nowMillis);
} }
// This method constructs a Peer and puts it into pendingPeers. // This method constructs a Peer and puts it into pendingPeers.
checkNotNull(addr); // Help static analysis which can't see that addr is always set if we didn't throw above.
connectTo(addr, false); connectTo(addr, false);
} }