Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve finding peers #7626

Merged
merged 18 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
remove last seen and last contacted, as they are not used
Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>
  • Loading branch information
pinges committed Sep 30, 2024
commit e8a03969a09887e66404994eb6b57390618d4520
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
import org.hyperledger.besu.ethereum.forkid.ForkId;
import org.hyperledger.besu.ethereum.p2p.peers.DefaultPeer;
import org.hyperledger.besu.ethereum.p2p.peers.Peer;
import org.hyperledger.besu.ethereum.p2p.peers.PeerId;
import org.hyperledger.besu.ethereum.rlp.RLPInput;
import org.hyperledger.besu.ethereum.rlp.RLPOutput;
import org.hyperledger.besu.plugin.data.EnodeURL;

import java.util.Optional;
import java.util.concurrent.atomic.AtomicLong;

import org.apache.tuweni.bytes.Bytes;
import org.ethereum.beacon.discovery.schema.NodeRecord;
Expand All @@ -37,9 +37,7 @@ public class DiscoveryPeer extends DefaultPeer {
private final Endpoint endpoint;

// Timestamps.
private long firstDiscovered = 0;
private long lastContacted = 0;
private long lastSeen = 0;
private final AtomicLong firstDiscovered = new AtomicLong(0L);
private long lastAttemptedConnection = 0;

private NodeRecord nodeRecord;
Expand Down Expand Up @@ -96,20 +94,11 @@ public void setStatus(final PeerDiscoveryStatus status) {
}

public long getFirstDiscovered() {
return firstDiscovered;
return firstDiscovered.get();
}

public PeerId setFirstDiscovered(final long firstDiscovered) {
this.firstDiscovered = firstDiscovered;
return this;
}

public long getLastContacted() {
return lastContacted;
}

public void setLastContacted(final long lastContacted) {
this.lastContacted = lastContacted;
public void setFirstDiscovered(final long firstDiscovered) {
this.firstDiscovered.compareAndExchange(0L, firstDiscovered);
}

public long getLastAttemptedConnection() {
Expand All @@ -120,14 +109,6 @@ public void setLastAttemptedConnection(final long lastAttemptedConnection) {
this.lastAttemptedConnection = lastAttemptedConnection;
}

public long getLastSeen() {
return lastSeen;
}

public void setLastSeen(final long lastSeen) {
this.lastSeen = lastSeen;
}

public Endpoint getEndpoint() {
return endpoint;
}
Expand Down Expand Up @@ -163,8 +144,6 @@ public String toString() {
sb.append("status=").append(status);
sb.append(", enode=").append(this.getEnodeURL());
sb.append(", firstDiscovered=").append(firstDiscovered);
sb.append(", lastContacted=").append(lastContacted);
sb.append(", lastSeen=").append(lastSeen);
sb.append('}');
return sb.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,9 +365,7 @@ protected void handleOutgoingPacket(final DiscoveryPeer peer, final Packet packe
(res, err) -> {
if (err != null) {
handleOutgoingPacketError(err, peer, packet);
return;
}
peer.setLastContacted(System.currentTimeMillis());
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,6 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) {
switch (packet.getType()) {
case PING:
if (peerPermissions.allowInboundBonding(peer)) {
peer.setLastSeen(System.currentTimeMillis());
final PingPacketData ping = packet.getPacketData(PingPacketData.class).get();
if (!PeerDiscoveryStatus.BONDED.equals(peer.getStatus())
&& (bondingPeers.getIfPresent(sender.getId()) == null)) {
Expand Down Expand Up @@ -412,7 +411,7 @@ private void checkBeforeAddingToPeerTable(final DiscoveryPeer peer) {
return;
}

if (peer.getFirstDiscovered() == 0) {
if (peer.getFirstDiscovered() == 0L) {
connectOnRlpxLayer(peer)
.whenComplete(
(pc, th) -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,69 +40,20 @@ public void lastSeenAndFirstDiscoveredTimestampsUpdatedOnMessage() {
final Packet pong = helper.createPongPacket(agent, Hash.hash(agentPing.getHash()));
helper.sendMessageBetweenAgents(testAgent, agent, pong);

long lastSeen;
long firstDiscovered;

assertThat(agent.streamDiscoveredPeers()).hasSize(1);

DiscoveryPeer p = agent.streamDiscoveredPeers().iterator().next();
assertThat(p.getLastSeen()).isGreaterThan(0);
assertThat(p.getFirstDiscovered()).isGreaterThan(0);

lastSeen = p.getLastSeen();
firstDiscovered = p.getFirstDiscovered();

helper.sendMessageBetweenAgents(testAgent, agent, testAgentPing);

assertThat(agent.streamDiscoveredPeers()).hasSize(1);

p = agent.streamDiscoveredPeers().iterator().next();
assertThat(p.getLastSeen()).isGreaterThan(lastSeen);
assertThat(p.getFirstDiscovered()).isEqualTo(firstDiscovered);
}

@Test
public void lastContactedTimestampUpdatedOnOutboundMessage() {
final MockPeerDiscoveryAgent agent = helper.startDiscoveryAgent(Collections.emptyList());
assertThat(agent.streamDiscoveredPeers()).hasSize(0);

// Start a test peer and send a PING packet to the agent under test.
final MockPeerDiscoveryAgent testAgent = helper.startDiscoveryAgent();
final Packet ping = helper.createPingPacket(testAgent, agent);
helper.sendMessageBetweenAgents(testAgent, agent, ping);

assertThat(agent.streamDiscoveredPeers()).hasSize(1);

final long lastContacted;
final long lastSeen;
final long firstDiscovered;

DiscoveryPeer peer = agent.streamDiscoveredPeers().iterator().next();
final long lc = peer.getLastContacted();
final long ls = peer.getLastSeen();
final long fd = peer.getFirstDiscovered();

assertThat(lc).isGreaterThan(0);
assertThat(ls).isGreaterThan(0);
assertThat(fd).isGreaterThan(0);

lastContacted = lc;
lastSeen = ls;
firstDiscovered = fd;

// Send another packet and ensure that timestamps are updated accordingly.
// Sleep beforehand to make sure timestamps will be different.
try {
Thread.sleep(1);
} catch (InterruptedException e) {
// Swallow exception because we only want to pause the test.
}
helper.sendMessageBetweenAgents(testAgent, agent, ping);

peer = agent.streamDiscoveredPeers().iterator().next();

assertThat(peer.getLastContacted()).isGreaterThan(lastContacted);
assertThat(peer.getLastSeen()).isGreaterThan(lastSeen);
assertThat(peer.getFirstDiscovered()).isEqualTo(firstDiscovered);
}
}