diff --git a/src/freenet/clients/http/DarknetConnectionsToadlet.java b/src/freenet/clients/http/DarknetConnectionsToadlet.java index cb8d4972d7a..5a407b34d04 100644 --- a/src/freenet/clients/http/DarknetConnectionsToadlet.java +++ b/src/freenet/clients/http/DarknetConnectionsToadlet.java @@ -394,7 +394,7 @@ protected void handleAltPost(URI uri, HTTPRequest request, ToadletContext ctx, b DarknetPeerNode[] peerNodes = node.getDarknetConnections(); for(DarknetPeerNode pn: peerNodes) { if (request.isPartSet("node_"+pn.hashCode())) { - if((pn.timeLastConnectionCompleted() < (System.currentTimeMillis() - 1000*60*60*24*7) /* one week */) || (pn.peerNodeStatus == PeerManager.PEER_NODE_STATUS_NEVER_CONNECTED) || request.isPartSet("forceit")){ + if((pn.timeLastConnectionCompleted() < (System.currentTimeMillis() - 1000*60*60*24*7) /* one week */) || (pn.getPeerNodeStatus() == PeerManager.PEER_NODE_STATUS_NEVER_CONNECTED) || request.isPartSet("forceit")){ this.node.removePeerConnection(pn); if(logMINOR) Logger.minor(this, "Removed node: node_"+pn.hashCode()); }else{ diff --git a/src/freenet/node/CHKInsertHandler.java b/src/freenet/node/CHKInsertHandler.java index bf5282f2cd7..6b16983bb15 100644 --- a/src/freenet/node/CHKInsertHandler.java +++ b/src/freenet/node/CHKInsertHandler.java @@ -444,7 +444,22 @@ private void finish(int code) { // Ignore. } - Logger.error(this, "Insert took too long, telling downstream that it's finished and reassigning to self on "+this); + /* Our predecessor expects us to timeout within the fixed timeout time. However + * it takes some time to forward the request, and we might have sent it to several + * nodes, so it's possible that CHKInsertSender is still waiting for downstream + * nodes to reply even though our predecessor has timed out. This will happen any + * time the last node fails to respond. + * + * Also, for load management purposes, it is important to know exactly how many + * of our requests are running, regardless of timeouts (especially with NLM). + * + * The solution implemented here is to take ownership of the insert ourself and + * tell our predecessor that we've timed out. + * + * FIXME Reconsider. + * + * See comments on TRANSFER_COMPLETION_ACK_TIMEOUT_BULK etc. */ + Logger.normal(this, "Insert took too long, telling downstream that it's finished and reassigning to self on "+this); // Still waiting. while(true) { diff --git a/src/freenet/node/CHKInsertSender.java b/src/freenet/node/CHKInsertSender.java index 6f177a0643f..d8f371af45e 100644 --- a/src/freenet/node/CHKInsertSender.java +++ b/src/freenet/node/CHKInsertSender.java @@ -340,7 +340,15 @@ void start() { } // Constants + /* Time for a node to accept an insert */ static final long ACCEPTED_TIMEOUT = SECONDS.toMillis(10); + /* We have a fixed timeout. This causes some problems, since if the last node on the + * chain fails, the first node to reach its timeout will be the originator (htl 18), + * because it started counting first! In 0.5 the timeout depended on the HTL, but the + * HTL does not strictly decrease (we spend several hops at HTL 18 and HTL 1). + * FIXME Reconsider. + * + * There are related complexities in CHKInsertHandler. */ static final long TRANSFER_COMPLETION_ACK_TIMEOUT_REALTIME = MINUTES.toMillis(1); static final long TRANSFER_COMPLETION_ACK_TIMEOUT_BULK = MINUTES.toMillis(5); diff --git a/src/freenet/node/DarknetPeerNode.java b/src/freenet/node/DarknetPeerNode.java index 16629aa4823..174450de512 100644 --- a/src/freenet/node/DarknetPeerNode.java +++ b/src/freenet/node/DarknetPeerNode.java @@ -300,11 +300,11 @@ public synchronized String getName() { } @Override - protected synchronized int getPeerNodeStatus(long now, long backedOffUntilRT, long backedOffUntilBulk, boolean overPingThreshold, boolean noLoadStats) { + protected synchronized int calculatePeerNodeStatus(long now, long backedOffUntilRT, long backedOffUntilBulk, boolean overPingThreshold, boolean noLoadStats) { if(isDisabled) { return PeerManager.PEER_NODE_STATUS_DISABLED; } - int status = super.getPeerNodeStatus(now, backedOffUntilRT, backedOffUntilBulk, overPingThreshold, noLoadStats); + int status = super.calculatePeerNodeStatus(now, backedOffUntilRT, backedOffUntilBulk, overPingThreshold, noLoadStats); if(status == PeerManager.PEER_NODE_STATUS_CONNECTED || status == PeerManager.PEER_NODE_STATUS_CLOCK_PROBLEM || status == PeerManager.PEER_NODE_STATUS_ROUTING_BACKED_OFF || diff --git a/src/freenet/node/Node.java b/src/freenet/node/Node.java index cc4f1861243..34392c9ca26 100644 --- a/src/freenet/node/Node.java +++ b/src/freenet/node/Node.java @@ -4416,7 +4416,7 @@ public int getUnclaimedFIFOSize() { * Connect this node to another node (for purposes of testing) */ public void connectToSeednode(SeedServerTestPeerNode node) throws OpennetDisabledException, FSParseException, PeerParseException, ReferenceSignatureVerificationException { - peers.addPeer(node,false,false); + peers.addPeer(node,false); } public void connect(Node node, FRIEND_TRUST trust, FRIEND_VISIBILITY visibility) throws FSParseException, PeerParseException, ReferenceSignatureVerificationException { peers.connect(node.darknetCrypto.exportPublicFieldSet(), darknetCrypto.packetMangler, trust, visibility); diff --git a/src/freenet/node/NodeCrypto.java b/src/freenet/node/NodeCrypto.java index cc2f6ee39f4..0183e8636a8 100644 --- a/src/freenet/node/NodeCrypto.java +++ b/src/freenet/node/NodeCrypto.java @@ -592,6 +592,7 @@ public void maybeBootConnection(PeerNode peerNode, for(PeerNode pn : possibleMatches) { if(pn == peerNode) continue; if(pn.equals(peerNode)) continue; + if(logMINOR) Logger.minor(this, "Booting connection "+peerNode+" because have "+pn+" on same IP"); if(pn.crypto.config.oneConnectionPerAddress()) { if(pn instanceof DarknetPeerNode) { if(!(peerNode instanceof DarknetPeerNode)) { diff --git a/src/freenet/node/OpennetManager.java b/src/freenet/node/OpennetManager.java index 29f992a95d7..6c1d221195c 100644 --- a/src/freenet/node/OpennetManager.java +++ b/src/freenet/node/OpennetManager.java @@ -612,7 +612,7 @@ public boolean wantPeer(OpennetPeerNode nodeToAddNow, boolean addAtLRU, boolean nodeToAddNow.setAddedReason(connectionType); if(notMany) { if(nodeToAddNow != null) { - node.peers.addPeer(nodeToAddNow, true, true); // Add to peers outside the OM lock + node.peers.addPeer(nodeToAddNow, true); // Add to peers outside the OM lock } return true; } @@ -691,7 +691,7 @@ public boolean wantPeer(OpennetPeerNode nodeToAddNow, boolean addAtLRU, boolean } } } - if(nodeToAddNow != null && canAdd && !node.peers.addPeer(nodeToAddNow, true, true)) { + if(nodeToAddNow != null && canAdd && !node.peers.addPeer(nodeToAddNow, true)) { if(logMINOR) Logger.minor(this, "Already in global peers list: "+nodeToAddNow+" when adding opennet node"); // Just because it's in the global peers list doesn't mean its in the LRU, it may be an old-opennet-peers reconnection. diff --git a/src/freenet/node/PeerManager.java b/src/freenet/node/PeerManager.java index 90d78923458..5bf7ebeec59 100644 --- a/src/freenet/node/PeerManager.java +++ b/src/freenet/node/PeerManager.java @@ -252,7 +252,7 @@ private boolean readPeers(File peersFile, OutgoingPacketMangler mangler, NodeCry else opennet.addOldOpennetNode((OpennetPeerNode)pn); } else - addPeer(pn, true, false); + addPeer(pn, true); gotSome = true; } catch(FSParseException e2) { Logger.error(this, "Could not parse peer: " + e2 + '\n' + fs.toString(), e2); @@ -305,7 +305,7 @@ private boolean readPeers(File peersFile, OutgoingPacketMangler mangler, NodeCry } public boolean addPeer(PeerNode pn) { - return addPeer(pn, false, false); + return addPeer(pn, false); } /** @@ -317,10 +317,10 @@ public boolean addPeer(PeerNode pn) { * @return True if the node was successfully added. False if it was already present, or if we tried to add * an opennet peer when opennet was disabled. */ - boolean addPeer(PeerNode pn, boolean ignoreOpennet, boolean reactivate) { + boolean addPeer(PeerNode pn, boolean ignoreOpennet) { assert (pn != null); - if(reactivate) - pn.forceCancelDisconnecting(); + // Call onAdded() first for the case where it is present but disconnecting. + pn.onAdded(); synchronized(this) { for(PeerNode myPeer: myPeers) { if(myPeer.equals(pn)) { @@ -333,9 +333,6 @@ boolean addPeer(PeerNode pn, boolean ignoreOpennet, boolean reactivate) { myPeers[myPeers.length - 1] = pn; Logger.normal(this, "Added " + pn); } - if(pn.recordStatus()) - addPeerNodeStatus(pn.getPeerNodeStatus(), pn, false); - pn.setPeerNodeStatus(System.currentTimeMillis()); if((!ignoreOpennet) && pn instanceof OpennetPeerNode) { OpennetManager opennet = node.getOpennet(); if(opennet != null) @@ -1800,11 +1797,9 @@ public void maybeLogPeerNodeStatusSummary(long now) { public void changePeerNodeStatus(PeerNode peerNode, int oldPeerNodeStatus, int peerNodeStatus, boolean noLog) { - Integer newStatus = Integer.valueOf(peerNodeStatus); - Integer oldStatus = Integer.valueOf(oldPeerNodeStatus); - this.allPeersStatuses.changePeerNodeStatus(peerNode, oldStatus, newStatus, noLog); + this.allPeersStatuses.changePeerNodeStatus(peerNode, oldPeerNodeStatus, peerNodeStatus, noLog); if(!peerNode.isOpennet()) - this.darknetPeersStatuses.changePeerNodeStatus(peerNode, oldStatus, newStatus, noLog); + this.darknetPeersStatuses.changePeerNodeStatus(peerNode, oldPeerNodeStatus, peerNodeStatus, noLog); node.executor.execute(new Runnable() { @Override diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 9e3c832b68b..9da88ffb24b 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -320,7 +320,7 @@ public abstract class PeerNode implements USKRetrieverCallback, BasePeerNode, Pe /** The time at which we last completed a connection setup. */ private long connectedTime; /** The status of this peer node in terms of Node.PEER_NODE_STATUS_* */ - public int peerNodeStatus = PeerManager.PEER_NODE_STATUS_DISCONNECTED; + private int peerNodeStatus = PeerManager.PEER_NODE_STATUS_DISCONNECTED; static final long CHECK_FOR_SWAPPED_TRACKERS_INTERVAL = FNPPacketMangler.SESSION_KEY_REKEYING_INTERVAL / 30; @@ -453,6 +453,7 @@ protected boolean ignoreLastGoodVersion() { * @param node2 The running Node we are part of. */ public PeerNode(SimpleFieldSet fs, Node node2, NodeCrypto crypto, PeerManager peers, boolean fromLocal, boolean fromAnonymousInitiator, OutgoingPacketMangler mangler, boolean isOpennet) throws FSParseException, PeerParseException, ReferenceSignatureVerificationException { + removed = true; boolean noSig = false; if(fromLocal || fromAnonymousInitiator) noSig = true; myRef = new WeakReference(this); @@ -1490,19 +1491,21 @@ public long lastSentPacketTime() { public boolean shouldSendHandshake() { long now = System.currentTimeMillis(); boolean tempShouldSendHandshake = false; + boolean disconnected; synchronized(this) { if(disconnecting) return false; - tempShouldSendHandshake = ((now > sendHandshakeTime) && (handshakeIPs != null) && (isRekeying || !isConnected())); + disconnected = !isConnected(); + tempShouldSendHandshake = ((now > sendHandshakeTime) && (handshakeIPs != null) && (isRekeying || disconnected)); } if(logMINOR) Logger.minor(this, "shouldSendHandshake(): initial = "+tempShouldSendHandshake); if(tempShouldSendHandshake && (hasLiveHandshake(now))) tempShouldSendHandshake = false; if(tempShouldSendHandshake) { - if(isBurstOnly()) { + if(disconnected && isBurstOnly()) { synchronized(this) { isBursting = true; } - setPeerNodeStatus(System.currentTimeMillis()); + setPeerNodeStatus(System.currentTimeMillis()); } else return true; } @@ -1539,7 +1542,7 @@ public boolean hasLiveHandshake(long now) { * Set sendHandshakeTime, and return whether to fetch the ARK. */ protected boolean innerCalcNextHandshake(boolean successfulHandshakeSend, boolean dontFetchARK, long now) { - if(isBurstOnly()) + if((!isConnected()) && isBurstOnly()) return calcNextHandshakeBurstOnly(now); synchronized(this) { long delay; @@ -1824,7 +1827,10 @@ public void updateLocation(double newLoc, double[] newLocs) { // Not urgent. This makes up the majority of the total writes. // Writing it on shutdown is sufficient. node.peers.writePeers(isOpennet()); - setPeerNodeStatus(System.currentTimeMillis()); + synchronized(this) { + if(!isConnected()) return; // Race condition. + setPeerNodeStatus(System.currentTimeMillis()); + } } /** @@ -3480,9 +3486,16 @@ else if(status == PeerManager.PEER_NODE_STATUS_NO_LOAD_STATS) return "peer_unknown_status"; } - protected synchronized int getPeerNodeStatus(long now, long routingBackedOffUntilRT, long localRoutingBackedOffUntilBulk, boolean overPingTime, boolean noLoadStats) { + /** Calculate the current status. Does not update peerNodeStatus itself, nor does it update + * the various listeners and trackers interested in peer statuses; see + * setPeerNodeStatus(long, boolean) for that. Override this method to implement special peer + * statuses that are specific to your PeerNode implementation (e.g. DarknetPeerNode). + * You should generally call this method and then adjust the return value in special cases. + * Note that this method updates the backoff statistics. */ + protected synchronized int calculatePeerNodeStatus(long now, long routingBackedOffUntilRT, long localRoutingBackedOffUntilBulk, boolean overPingTime, boolean noLoadStats) { if(disconnecting) return PeerManager.PEER_NODE_STATUS_DISCONNECTING; + int peerNodeStatus; // Caller's responsibility to update this.peerNodeStatus. boolean isConnected = isConnected(); if(isRoutable()) { // Function use also updates timeLastConnected and timeLastRoutable if(noLoadStats) @@ -3543,13 +3556,15 @@ else if(isBursting) return PeerManager.PEER_NODE_STATUS_BURSTING; else peerNodeStatus = PeerManager.PEER_NODE_STATUS_DISCONNECTED; - if(!isConnected && (previousRoutingBackoffReasonRT != null)) { - peers.removePeerNodeRoutingBackoffReason(previousRoutingBackoffReasonRT, this, true); - previousRoutingBackoffReasonRT = null; - } - if(!isConnected && (previousRoutingBackoffReasonBulk != null)) { - peers.removePeerNodeRoutingBackoffReason(previousRoutingBackoffReasonBulk, this, false); - previousRoutingBackoffReasonBulk = null; + if(!removed) { + if(!isConnected && (previousRoutingBackoffReasonRT != null)) { + peers.removePeerNodeRoutingBackoffReason(previousRoutingBackoffReasonRT, this, true); + previousRoutingBackoffReasonRT = null; + } + if(!isConnected && (previousRoutingBackoffReasonBulk != null)) { + peers.removePeerNodeRoutingBackoffReason(previousRoutingBackoffReasonBulk, this, false); + previousRoutingBackoffReasonBulk = null; + } } return peerNodeStatus; } @@ -3558,7 +3573,12 @@ public int setPeerNodeStatus(long now) { return setPeerNodeStatus(now, false); } - public int setPeerNodeStatus(long now, boolean noLog) { + /* Calls calculatePeerNodeStatus() to calculate the current status, and then updates everyone + * who needs to know. In particular the PeerStatusTracker but also load management stuff and + * UI listeners. Should not be overridden in general. + * @param now The current time. + * @param noLog If true don't complain if a peer status e.g. isn't in the tracker. */ + public final int setPeerNodeStatus(long now, boolean noLog) { long localRoutingBackedOffUntilRT = getRoutingBackedOffUntil(true); long localRoutingBackedOffUntilBulk = getRoutingBackedOffUntil(true); int oldPeerNodeStatus; @@ -3566,9 +3586,9 @@ public int setPeerNodeStatus(long now, boolean noLog) { boolean noLoadStats = noLoadStats(); synchronized(this) { oldPeerNodeStatus = peerNodeStatus; - peerNodeStatus = getPeerNodeStatus(now, localRoutingBackedOffUntilRT, localRoutingBackedOffUntilBulk, averagePingTime() > threshold, noLoadStats); + peerNodeStatus = calculatePeerNodeStatus(now, localRoutingBackedOffUntilRT, localRoutingBackedOffUntilBulk, averagePingTime() > threshold, noLoadStats); - if(peerNodeStatus != oldPeerNodeStatus && recordStatus()) { + if(peerNodeStatus != oldPeerNodeStatus && recordStatus() && !removed) { peers.changePeerNodeStatus(this, oldPeerNodeStatus, peerNodeStatus, noLog); } @@ -3935,11 +3955,9 @@ public boolean notifyDisconnecting(boolean dumpMessageQueue) { /** Called to cancel a delayed disconnect. Always succeeds even if the node was not being * disconnected. */ - public void forceCancelDisconnecting() { + public void onAdded() { synchronized(this) { removed = false; - if(!disconnecting) - return; disconnecting = false; } setPeerNodeStatus(System.currentTimeMillis(), true); diff --git a/src/freenet/node/PeerStatusTracker.java b/src/freenet/node/PeerStatusTracker.java index 3bb90c1921d..2fcb73dc6d7 100644 --- a/src/freenet/node/PeerStatusTracker.java +++ b/src/freenet/node/PeerStatusTracker.java @@ -11,7 +11,7 @@ class PeerStatusTracker { private static volatile boolean logMINOR; static { - Logger.registerClass(PeerManager.class); + Logger.registerClass(PeerStatusTracker.class); } /** PeerNode statuses, by status. WARNING: LOCK THIS LAST. Must NOT call PeerNode inside this lock. */ @@ -29,13 +29,13 @@ public synchronized void addStatus(K peerNodeStatus, PeerNode peerNode, boolean Logger.error(this, "addPeerNodeStatus(): node already in peerNodeStatuses: " + peerNode + " status " + peerNodeStatus, new Exception("debug")); return; } - statuses.remove(peerNodeStatus); - } else + } else { statusSet = new WeakHashSet(); + statuses.put(peerNodeStatus, statusSet); + } if(logMINOR) - Logger.minor(this, "addPeerNodeStatus(): adding PeerNode for '" + peerNode.getIdentityString() + "' with status '" + peerNodeStatus + "'"); + Logger.minor(this, "addPeerNodeStatus(): adding PeerNode for '" + peerNode.getIdentityString() + "' with status '" + peerNodeStatus + "'", new Exception("debug")); statusSet.add(peerNode); - statuses.put(peerNodeStatus, statusSet); } public synchronized int statusSize(K pnStatus) { @@ -59,7 +59,7 @@ public synchronized void removeStatus(K peerNodeStatus, PeerNode peerNode, statuses.remove(peerNodeStatus); } if(logMINOR) - Logger.minor(this, "removePeerNodeStatus(): removing PeerNode for '" + peerNode.getIdentityString() + "' with status '" + peerNodeStatus + "'"); + Logger.minor(this, "removePeerNodeStatus(): removing PeerNode for '" + peerNode.getIdentityString() + "' with status '" + peerNodeStatus + "'", new Exception("debug")); } public synchronized void changePeerNodeStatus(PeerNode peerNode, K oldPeerNodeStatus,