From 1e50e2e090f395cdb0251cea41e00366fd6d61f1 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Wed, 12 Aug 2015 21:09:28 +0100 Subject: [PATCH 01/21] If connected, ignore burst only setting This should fix some annoying log spam and rekey quicker. --- src/freenet/node/PeerNode.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 9e3c832b68b..1b6d77e67c5 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -1490,19 +1490,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 +1541,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; From 827b815e55c91a09228304e6337ffc0a0cf6fd87 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Wed, 12 Aug 2015 21:34:08 +0100 Subject: [PATCH 02/21] Cleanup: Use auto-boxing --- src/freenet/node/PeerManager.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/freenet/node/PeerManager.java b/src/freenet/node/PeerManager.java index 90d78923458..cb8ffbcfb4e 100644 --- a/src/freenet/node/PeerManager.java +++ b/src/freenet/node/PeerManager.java @@ -1800,11 +1800,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 From 8b39df66b6fc0a5d0019569da9ce8850a165fe50 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Wed, 12 Aug 2015 21:38:57 +0100 Subject: [PATCH 03/21] Should be private --- src/freenet/clients/http/DarknetConnectionsToadlet.java | 2 +- src/freenet/node/PeerNode.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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/PeerNode.java b/src/freenet/node/PeerNode.java index 1b6d77e67c5..802414840ff 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; From fdcd4bc78463b9ce3016a819eb89897c75177ae3 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Wed, 12 Aug 2015 21:43:38 +0100 Subject: [PATCH 04/21] Update the status consistently. Should fix bogus logging on nodes with darknet peers. --- src/freenet/node/PeerNode.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 802414840ff..245bdb99534 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3482,9 +3482,12 @@ else if(status == PeerManager.PEER_NODE_STATUS_NO_LOAD_STATS) return "peer_unknown_status"; } + /** Calculate the current status. Do not update this.peerNodeStatus and do not update the + * PeerManager. */ protected synchronized int getPeerNodeStatus(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) From d95d01bca53176b4ec433562a340855716415d4a Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Wed, 12 Aug 2015 22:26:18 +0100 Subject: [PATCH 05/21] Fix logMINOR --- src/freenet/node/PeerStatusTracker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/freenet/node/PeerStatusTracker.java b/src/freenet/node/PeerStatusTracker.java index 3bb90c1921d..3731585367c 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. */ From 99d9462342ad4dda41b981e666980f6a567baff9 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Wed, 12 Aug 2015 22:34:25 +0100 Subject: [PATCH 06/21] Cleanup wierd code (but was still correct) --- src/freenet/node/PeerStatusTracker.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/freenet/node/PeerStatusTracker.java b/src/freenet/node/PeerStatusTracker.java index 3731585367c..bcb1cdee541 100644 --- a/src/freenet/node/PeerStatusTracker.java +++ b/src/freenet/node/PeerStatusTracker.java @@ -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 + "'"); statusSet.add(peerNode); - statuses.put(peerNodeStatus, statusSet); } public synchronized int statusSize(K pnStatus) { From f98ac5875ec55122460b60805a564e5104e01d88 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Wed, 12 Aug 2015 23:24:00 +0100 Subject: [PATCH 07/21] Don't include old opennet peers in the status table. This fixes logger errors due to trying to reconnect to old opennet peers which are not in the routing table and not in the status table either, but get set to BURSTING on non-NATed nodes. --- src/freenet/node/DarknetPeerNode.java | 6 ++++-- src/freenet/node/PacketSender.java | 2 +- src/freenet/node/PeerNode.java | 14 +++++++------- src/freenet/node/PeerNodeBackoffStatusChecker.java | 2 +- src/freenet/node/SeedClientPeerNode.java | 2 +- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/freenet/node/DarknetPeerNode.java b/src/freenet/node/DarknetPeerNode.java index 16629aa4823..3866ca17395 100644 --- a/src/freenet/node/DarknetPeerNode.java +++ b/src/freenet/node/DarknetPeerNode.java @@ -244,11 +244,13 @@ public synchronized Peer getPeer(){ * attempt. */ @Override - public boolean shouldSendHandshake() { + public boolean shouldSendHandshake(boolean notInRoutingTable) { + if(notInRoutingTable) + Logger.error(this, "Darknet peer not in routing table?!", new Exception("error")); synchronized(this) { if(isDisabled) return false; if(isListenOnly) return false; - if(!super.shouldSendHandshake()) return false; + if(!super.shouldSendHandshake(notInRoutingTable)) return false; } return true; } diff --git a/src/freenet/node/PacketSender.java b/src/freenet/node/PacketSender.java index f7f96f380b0..1ce35dd2142 100644 --- a/src/freenet/node/PacketSender.java +++ b/src/freenet/node/PacketSender.java @@ -428,7 +428,7 @@ private void realRun() { pn.startARKFetcher(); continue; } - if(pn.shouldSendHandshake()) { + if(pn.shouldSendHandshake(true)) { // Send handshake if necessary long beforeHandshakeTime = System.currentTimeMillis(); pn.getOutgoingMangler().sendHandshake(pn, true); diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 245bdb99534..749f3f4ff09 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -1487,7 +1487,7 @@ public long lastSentPacketTime() { * sufficient time period since we last sent a handshake * attempt. */ - public boolean shouldSendHandshake() { + public boolean shouldSendHandshake(boolean notInRoutingTable) { long now = System.currentTimeMillis(); boolean tempShouldSendHandshake = false; boolean disconnected; @@ -1504,7 +1504,7 @@ public boolean shouldSendHandshake() { synchronized(this) { isBursting = true; } - setPeerNodeStatus(System.currentTimeMillis()); + setPeerNodeStatus(System.currentTimeMillis(), false, notInRoutingTable); } else return true; } @@ -3560,10 +3560,10 @@ else if(isBursting) } public int setPeerNodeStatus(long now) { - return setPeerNodeStatus(now, false); + return setPeerNodeStatus(now, false, false); } - public int setPeerNodeStatus(long now, boolean noLog) { + public int setPeerNodeStatus(long now, boolean noLog, boolean temporaryNoStatus) { long localRoutingBackedOffUntilRT = getRoutingBackedOffUntil(true); long localRoutingBackedOffUntilBulk = getRoutingBackedOffUntil(true); int oldPeerNodeStatus; @@ -3573,7 +3573,7 @@ public int setPeerNodeStatus(long now, boolean noLog) { oldPeerNodeStatus = peerNodeStatus; peerNodeStatus = getPeerNodeStatus(now, localRoutingBackedOffUntilRT, localRoutingBackedOffUntilBulk, averagePingTime() > threshold, noLoadStats); - if(peerNodeStatus != oldPeerNodeStatus && recordStatus()) { + if(peerNodeStatus != oldPeerNodeStatus && recordStatus() && !temporaryNoStatus) { peers.changePeerNodeStatus(this, oldPeerNodeStatus, peerNodeStatus, noLog); } @@ -3947,7 +3947,7 @@ public void forceCancelDisconnecting() { return; disconnecting = false; } - setPeerNodeStatus(System.currentTimeMillis(), true); + setPeerNodeStatus(System.currentTimeMillis(), true, false); } /** Called when the peer is removed from the PeerManager */ @@ -4056,7 +4056,7 @@ public WeakReference getWeakRef() { */ public Peer getHandshakeIP() { Peer[] localHandshakeIPs; - if(!shouldSendHandshake()) { + if(!shouldSendHandshake(false)) { if(logMINOR) Logger.minor(this, "Not sending handshake to "+getPeer()+" because pn.shouldSendHandshake() returned false"); return null; } diff --git a/src/freenet/node/PeerNodeBackoffStatusChecker.java b/src/freenet/node/PeerNodeBackoffStatusChecker.java index b008d7c80bf..1217026e88e 100644 --- a/src/freenet/node/PeerNodeBackoffStatusChecker.java +++ b/src/freenet/node/PeerNodeBackoffStatusChecker.java @@ -40,6 +40,6 @@ public void run() { Logger.error(this, "Not in peers table but not flagged as removed: "+pn); return; } - pn.setPeerNodeStatus(System.currentTimeMillis(), true); + pn.setPeerNodeStatus(System.currentTimeMillis(), true, false); } } diff --git a/src/freenet/node/SeedClientPeerNode.java b/src/freenet/node/SeedClientPeerNode.java index 0bc25a6aafc..59a11f9371d 100644 --- a/src/freenet/node/SeedClientPeerNode.java +++ b/src/freenet/node/SeedClientPeerNode.java @@ -86,7 +86,7 @@ public int handshakeSetupType() { } @Override - public boolean shouldSendHandshake() { + public boolean shouldSendHandshake(boolean ignored) { return false; } From 18985144ee751b42432456b2548de0a2b23667f7 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Fri, 14 Aug 2015 17:09:24 +0100 Subject: [PATCH 08/21] Logging --- src/freenet/node/NodeCrypto.java | 1 + 1 file changed, 1 insertion(+) 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)) { From 8379f1b2615a9f8071c144da86e7c57eb1d6043b Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Fri, 14 Aug 2015 17:17:49 +0100 Subject: [PATCH 09/21] Fix race condition leading to bogus logging --- src/freenet/node/PeerNode.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 749f3f4ff09..48d6ac1a7ba 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -1826,7 +1826,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()); + } } /** From f9b5cdb5ba016531d2db3d614dc5158b06dd2178 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 13:02:51 +0100 Subject: [PATCH 10/21] Don't update status if not in the peers table. Mainly for squashing log errors, but the peer counts *are* used for important things, notably determining whether we need more opennet peers. --- src/freenet/node/PeerNode.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 9e3c832b68b..acdad8739e2 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3568,7 +3568,7 @@ public int setPeerNodeStatus(long now, boolean noLog) { oldPeerNodeStatus = peerNodeStatus; peerNodeStatus = getPeerNodeStatus(now, localRoutingBackedOffUntilRT, localRoutingBackedOffUntilBulk, averagePingTime() > threshold, noLoadStats); - if(peerNodeStatus != oldPeerNodeStatus && recordStatus()) { + if(peerNodeStatus != oldPeerNodeStatus && recordStatus() && !removed) { peers.changePeerNodeStatus(this, oldPeerNodeStatus, peerNodeStatus, noLog); } From 78728172f14f799dc1fc306a256e615f528905c4 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 13:03:29 +0100 Subject: [PATCH 11/21] Logging --- src/freenet/node/PeerStatusTracker.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/freenet/node/PeerStatusTracker.java b/src/freenet/node/PeerStatusTracker.java index 3bb90c1921d..3b1c5178ab2 100644 --- a/src/freenet/node/PeerStatusTracker.java +++ b/src/freenet/node/PeerStatusTracker.java @@ -33,7 +33,7 @@ public synchronized void addStatus(K peerNodeStatus, PeerNode peerNode, boolean } else statusSet = new WeakHashSet(); 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); } @@ -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, From 71befe86fec89196265f3c913b835d4ecd0bd7e0 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 13:18:00 +0100 Subject: [PATCH 12/21] Revert "Don't include old opennet peers in the status table." This reverts commit f98ac5875ec55122460b60805a564e5104e01d88. This is wrong. There is a better way... Conflicts: src/freenet/node/PeerNode.java --- src/freenet/node/DarknetPeerNode.java | 6 ++---- src/freenet/node/PacketSender.java | 2 +- src/freenet/node/PeerNode.java | 14 +++++++------- src/freenet/node/PeerNodeBackoffStatusChecker.java | 2 +- src/freenet/node/SeedClientPeerNode.java | 2 +- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/freenet/node/DarknetPeerNode.java b/src/freenet/node/DarknetPeerNode.java index 3866ca17395..16629aa4823 100644 --- a/src/freenet/node/DarknetPeerNode.java +++ b/src/freenet/node/DarknetPeerNode.java @@ -244,13 +244,11 @@ public synchronized Peer getPeer(){ * attempt. */ @Override - public boolean shouldSendHandshake(boolean notInRoutingTable) { - if(notInRoutingTable) - Logger.error(this, "Darknet peer not in routing table?!", new Exception("error")); + public boolean shouldSendHandshake() { synchronized(this) { if(isDisabled) return false; if(isListenOnly) return false; - if(!super.shouldSendHandshake(notInRoutingTable)) return false; + if(!super.shouldSendHandshake()) return false; } return true; } diff --git a/src/freenet/node/PacketSender.java b/src/freenet/node/PacketSender.java index 1ce35dd2142..f7f96f380b0 100644 --- a/src/freenet/node/PacketSender.java +++ b/src/freenet/node/PacketSender.java @@ -428,7 +428,7 @@ private void realRun() { pn.startARKFetcher(); continue; } - if(pn.shouldSendHandshake(true)) { + if(pn.shouldSendHandshake()) { // Send handshake if necessary long beforeHandshakeTime = System.currentTimeMillis(); pn.getOutgoingMangler().sendHandshake(pn, true); diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index d33019a8f60..7fc7e35a0c7 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -1487,7 +1487,7 @@ public long lastSentPacketTime() { * sufficient time period since we last sent a handshake * attempt. */ - public boolean shouldSendHandshake(boolean notInRoutingTable) { + public boolean shouldSendHandshake() { long now = System.currentTimeMillis(); boolean tempShouldSendHandshake = false; boolean disconnected; @@ -1504,7 +1504,7 @@ public boolean shouldSendHandshake(boolean notInRoutingTable) { synchronized(this) { isBursting = true; } - setPeerNodeStatus(System.currentTimeMillis(), false, notInRoutingTable); + setPeerNodeStatus(System.currentTimeMillis()); } else return true; } @@ -3563,10 +3563,10 @@ else if(isBursting) } public int setPeerNodeStatus(long now) { - return setPeerNodeStatus(now, false, false); + return setPeerNodeStatus(now, false); } - public int setPeerNodeStatus(long now, boolean noLog, boolean temporaryNoStatus) { + public int setPeerNodeStatus(long now, boolean noLog) { long localRoutingBackedOffUntilRT = getRoutingBackedOffUntil(true); long localRoutingBackedOffUntilBulk = getRoutingBackedOffUntil(true); int oldPeerNodeStatus; @@ -3576,7 +3576,7 @@ public int setPeerNodeStatus(long now, boolean noLog, boolean temporaryNoStatus) oldPeerNodeStatus = peerNodeStatus; peerNodeStatus = getPeerNodeStatus(now, localRoutingBackedOffUntilRT, localRoutingBackedOffUntilBulk, averagePingTime() > threshold, noLoadStats); - if(peerNodeStatus != oldPeerNodeStatus && recordStatus() && !temporaryNoStatus && !removed) { + if(peerNodeStatus != oldPeerNodeStatus && recordStatus() && !removed) { peers.changePeerNodeStatus(this, oldPeerNodeStatus, peerNodeStatus, noLog); } @@ -3950,7 +3950,7 @@ public void forceCancelDisconnecting() { return; disconnecting = false; } - setPeerNodeStatus(System.currentTimeMillis(), true, false); + setPeerNodeStatus(System.currentTimeMillis(), true); } /** Called when the peer is removed from the PeerManager */ @@ -4059,7 +4059,7 @@ public WeakReference getWeakRef() { */ public Peer getHandshakeIP() { Peer[] localHandshakeIPs; - if(!shouldSendHandshake(false)) { + if(!shouldSendHandshake()) { if(logMINOR) Logger.minor(this, "Not sending handshake to "+getPeer()+" because pn.shouldSendHandshake() returned false"); return null; } diff --git a/src/freenet/node/PeerNodeBackoffStatusChecker.java b/src/freenet/node/PeerNodeBackoffStatusChecker.java index 1217026e88e..b008d7c80bf 100644 --- a/src/freenet/node/PeerNodeBackoffStatusChecker.java +++ b/src/freenet/node/PeerNodeBackoffStatusChecker.java @@ -40,6 +40,6 @@ public void run() { Logger.error(this, "Not in peers table but not flagged as removed: "+pn); return; } - pn.setPeerNodeStatus(System.currentTimeMillis(), true, false); + pn.setPeerNodeStatus(System.currentTimeMillis(), true); } } diff --git a/src/freenet/node/SeedClientPeerNode.java b/src/freenet/node/SeedClientPeerNode.java index 59a11f9371d..0bc25a6aafc 100644 --- a/src/freenet/node/SeedClientPeerNode.java +++ b/src/freenet/node/SeedClientPeerNode.java @@ -86,7 +86,7 @@ public int handshakeSetupType() { } @Override - public boolean shouldSendHandshake(boolean ignored) { + public boolean shouldSendHandshake() { return false; } From 67ce2693ce6a78db1d9bc0456244667dfdb56b76 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 13:31:48 +0100 Subject: [PATCH 13/21] PeerNode's start with removed=true, cleared on adding to RT This makes much more sense. It also fixes the "old opennet peers in the peer status tracker" problem more cleanly, and removes some unnecessary argument cruft. (Renames forceCancelDisconnecting to onAdded and always calls it in PeerManager.addPeer) --- src/freenet/node/Node.java | 2 +- src/freenet/node/OpennetManager.java | 4 ++-- src/freenet/node/PeerManager.java | 9 ++++----- src/freenet/node/PeerNode.java | 3 ++- 4 files changed, 9 insertions(+), 9 deletions(-) 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/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 cb8ffbcfb4e..46da1c7a512 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,9 @@ 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(); + pn.onAdded(); synchronized(this) { for(PeerNode myPeer: myPeers) { if(myPeer.equals(pn)) { diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 7fc7e35a0c7..834af73b7f7 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -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); @@ -3943,7 +3944,7 @@ 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) From 6b68c50d5bd814a407887578abfbdefd79f516de Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 13:39:36 +0100 Subject: [PATCH 14/21] Always add to status table (last commit not quite right) --- src/freenet/node/PeerNode.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 834af73b7f7..be475266570 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3947,8 +3947,6 @@ public boolean notifyDisconnecting(boolean dumpMessageQueue) { public void onAdded() { synchronized(this) { removed = false; - if(!disconnecting) - return; disconnecting = false; } setPeerNodeStatus(System.currentTimeMillis(), true); From a5d16e8b8d274ecec12d2761a8b0774c72ecab69 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 13:46:24 +0100 Subject: [PATCH 15/21] Rename and document --- src/freenet/node/DarknetPeerNode.java | 4 ++-- src/freenet/node/PeerNode.java | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) 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/PeerNode.java b/src/freenet/node/PeerNode.java index be475266570..2a66e242133 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3486,9 +3486,9 @@ else if(status == PeerManager.PEER_NODE_STATUS_NO_LOAD_STATS) return "peer_unknown_status"; } - /** Calculate the current status. Do not update this.peerNodeStatus and do not update the - * PeerManager. */ - protected synchronized int getPeerNodeStatus(long now, long routingBackedOffUntilRT, long localRoutingBackedOffUntilBulk, boolean overPingTime, boolean noLoadStats) { + /** Calculate the current status. Does not update peerNodeStatus itself, and does not update the status tracker. + * May update backoff statistics. Override this to implement special peer statuses (see DarknetPeerNode). */ + 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. @@ -3575,7 +3575,7 @@ 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() && !removed) { peers.changePeerNodeStatus(this, oldPeerNodeStatus, peerNodeStatus, noLog); From 5d1a48b205a31efdd27066327a098265eca30c15 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 13:50:13 +0100 Subject: [PATCH 16/21] Explain this method too. --- src/freenet/node/PeerNode.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index 2a66e242133..c2d4b36e72f 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3567,7 +3567,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; From 5287d768ded23d3f6871b416c3f9e7cc8bd632c2 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 13:52:58 +0100 Subject: [PATCH 17/21] Slightly better --- src/freenet/node/PeerNode.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index c2d4b36e72f..bad077e3947 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3486,8 +3486,12 @@ else if(status == PeerManager.PEER_NODE_STATUS_NO_LOAD_STATS) return "peer_unknown_status"; } - /** Calculate the current status. Does not update peerNodeStatus itself, and does not update the status tracker. - * May update backoff statistics. Override this to implement special peer statuses (see DarknetPeerNode). */ + /** 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; From 34e7b1b26bf4ea0bae92f0552bca4fe7172d2182 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 14:21:59 +0100 Subject: [PATCH 18/21] Another fix for peer status (simpler again) --- src/freenet/node/PeerManager.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/freenet/node/PeerManager.java b/src/freenet/node/PeerManager.java index 46da1c7a512..b12b589d8ec 100644 --- a/src/freenet/node/PeerManager.java +++ b/src/freenet/node/PeerManager.java @@ -319,7 +319,6 @@ public boolean addPeer(PeerNode pn) { */ boolean addPeer(PeerNode pn, boolean ignoreOpennet) { assert (pn != null); - pn.onAdded(); synchronized(this) { for(PeerNode myPeer: myPeers) { if(myPeer.equals(pn)) { @@ -332,9 +331,7 @@ boolean addPeer(PeerNode pn, boolean ignoreOpennet) { myPeers[myPeers.length - 1] = pn; Logger.normal(this, "Added " + pn); } - if(pn.recordStatus()) - addPeerNodeStatus(pn.getPeerNodeStatus(), pn, false); - pn.setPeerNodeStatus(System.currentTimeMillis()); + pn.onAdded(); if((!ignoreOpennet) && pn instanceof OpennetPeerNode) { OpennetManager opennet = node.getOpennet(); if(opennet != null) From 193abe0e3fe76514be589e6eb3e110869ba5c292 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 14:29:42 +0100 Subject: [PATCH 19/21] onAdded() must be called *before* the check. Fixes a minor regression in recent fixes. --- src/freenet/node/PeerManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/freenet/node/PeerManager.java b/src/freenet/node/PeerManager.java index b12b589d8ec..5bf7ebeec59 100644 --- a/src/freenet/node/PeerManager.java +++ b/src/freenet/node/PeerManager.java @@ -319,6 +319,8 @@ public boolean addPeer(PeerNode pn) { */ boolean addPeer(PeerNode pn, boolean ignoreOpennet) { assert (pn != null); + // Call onAdded() first for the case where it is present but disconnecting. + pn.onAdded(); synchronized(this) { for(PeerNode myPeer: myPeers) { if(myPeer.equals(pn)) { @@ -331,7 +333,6 @@ boolean addPeer(PeerNode pn, boolean ignoreOpennet) { myPeers[myPeers.length - 1] = pn; Logger.normal(this, "Added " + pn); } - pn.onAdded(); if((!ignoreOpennet) && pn instanceof OpennetPeerNode) { OpennetManager opennet = node.getOpennet(); if(opennet != null) From 7f5559c3b72c2f43118325469e89b1fccd74ed50 Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sat, 15 Aug 2015 17:52:20 +0100 Subject: [PATCH 20/21] Downgrade logging and explain the situation --- src/freenet/node/CHKInsertHandler.java | 17 ++++++++++++++++- src/freenet/node/CHKInsertSender.java | 8 ++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) 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); From 2166d3998cbd73012249070b9c90af35d2a3b99d Mon Sep 17 00:00:00 2001 From: Matthew Toseland Date: Sun, 16 Aug 2015 20:02:34 +0100 Subject: [PATCH 21/21] Don't remove if not in routing table. (Fixes more log spam) --- src/freenet/node/PeerNode.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/freenet/node/PeerNode.java b/src/freenet/node/PeerNode.java index bad077e3947..9da88ffb24b 100644 --- a/src/freenet/node/PeerNode.java +++ b/src/freenet/node/PeerNode.java @@ -3556,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; }