feat(snap): remove snap-state-chunk-specific timeout config

rsksmart · Dec 5, 2024 · 58edd8e · 58edd8e
1 parent f9ee554
commit 58edd8e
Show file tree

Hide file tree

Showing 9 changed files with 0 additions and 81 deletions.
diff --git a/rskj-core/src/main/java/co/rsk/RskContext.java b/rskj-core/src/main/java/co/rsk/RskContext.java
@@ -1521,7 +1521,6 @@ protected synchronized SyncConfiguration buildSyncConfiguration() {
                 rskSystemProperties.getTopBest(),
                 rskSystemProperties.isServerSnapshotSyncEnabled(),
                 rskSystemProperties.isClientSnapshotSyncEnabled(),
-                rskSystemProperties.getSnapshotChunkTimeout(),
                 rskSystemProperties.getSnapshotSyncLimit(),
                 rskSystemProperties.getSnapBootNodes());
     }

diff --git a/rskj-core/src/main/java/co/rsk/config/RskSystemProperties.java b/rskj-core/src/main/java/co/rsk/config/RskSystemProperties.java
@@ -32,7 +32,6 @@
 import org.ethereum.crypto.ECKey;
 import org.ethereum.crypto.HashUtil;
 import org.ethereum.listener.GasPriceCalculator;
-import org.ethereum.net.client.Capability;
 import org.ethereum.vm.PrecompiledContracts;
 
 import javax.annotation.Nonnull;
@@ -430,10 +429,6 @@ public int getLongSyncLimit() {
     public boolean isServerSnapshotSyncEnabled() { return configFromFiles.getBoolean("sync.snapshot.server.enabled");}
     public boolean isClientSnapshotSyncEnabled() { return configFromFiles.getBoolean(PROPERTY_SNAP_CLIENT_ENABLED);}
 
-    public int getSnapshotChunkTimeout() {
-        return configFromFiles.getInt("sync.snapshot.client.chunkRequestTimeout");
-    }
-
     public boolean isSnapshotParallelEnabled() { return configFromFiles.getBoolean("sync.snapshot.client.parallel");}
 
     public int getSnapshotChunkSize() { return configFromFiles.getInt("sync.snapshot.client.chunkSize");}

diff --git a/rskj-core/src/main/java/co/rsk/net/SnapshotProcessor.java b/rskj-core/src/main/java/co/rsk/net/SnapshotProcessor.java
@@ -600,7 +600,6 @@ void onStateChunkResponseError(Peer peer, SnapStateChunkResponseMessage response
     private void processOrderedStateChunkResponse(SnapSyncState state, Peer peer, SnapStateChunkResponseMessage message) throws Exception {
         logger.debug("Processing State chunk received from {} to {}", message.getFrom(), message.getTo());
         peersInformation.getOrRegisterPeer(peer);
-        state.onNewChunk();
 
         RLPList nodeLists = RLP.decodeList(message.getChunkOfTrieKeyValue());
         final RLPList preRootElements = RLP.decodeList(nodeLists.get(0).getRLPData());

diff --git a/rskj-core/src/main/java/co/rsk/net/sync/SnapSyncState.java b/rskj-core/src/main/java/co/rsk/net/sync/SnapSyncState.java
@@ -37,7 +37,6 @@
 
 import javax.annotation.Nullable;
 import java.math.BigInteger;
-import java.time.Duration;
 import java.util.*;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -177,20 +176,6 @@ public SyncEventsHandler getSyncEventsHandler() {
         return this.syncEventsHandler;
     }
 
-    public void onNewChunk() {
-        resetTimeElapsed();
-    }
-
-    @Override
-    public void tick(Duration duration) {
-        // TODO(snap-poc) handle multiple peers casuistry, similarly to co.rsk.net.sync.DownloadingBodiesSyncState.tick
-
-        timeElapsed = timeElapsed.plus(duration);
-        if (timeElapsed.compareTo(syncConfiguration.getTimeoutWaitingSnapChunk()) >= 0) {
-            onMessageTimeOut();
-        }
-    }
-
     @Override
     protected void onMessageTimeOut() {
         fail(getLastBlockSender(), EventType.TIMEOUT_MESSAGE, "Snap sync timed out");

diff --git a/rskj-core/src/main/java/co/rsk/net/sync/SyncConfiguration.java b/rskj-core/src/main/java/co/rsk/net/sync/SyncConfiguration.java
@@ -47,8 +47,6 @@ public final class SyncConfiguration {
     private final boolean isServerSnapSyncEnabled;
     private final boolean isClientSnapSyncEnabled;
 
-    private final Duration timeoutWaitingSnapChunk;
-
     private final int snapshotSyncLimit;
     private final Map<String, Node> nodeIdToSnapshotTrustedPeerMap;
 
@@ -92,7 +90,6 @@ public SyncConfiguration(
                 topBest,
                 isServerSnapSyncEnabled,
                 isClientSnapSyncEnabled,
-                timeoutWaitingSnapChunk,
                 snapshotSyncLimit,
                 Collections.emptyList());
     }
@@ -109,7 +106,6 @@ public SyncConfiguration(
             double topBest,
             boolean isServerSnapSyncEnabled,
             boolean isClientSnapSyncEnabled,
-            int timeoutWaitingSnapChunk,
             int snapshotSyncLimit,
             List<Node> snapBootNodes) {
         this.expectedPeers = expectedPeers;
@@ -123,8 +119,6 @@ public SyncConfiguration(
         this.topBest = topBest;
         this.isServerSnapSyncEnabled = isServerSnapSyncEnabled;
         this.isClientSnapSyncEnabled = isClientSnapSyncEnabled;
-        // TODO(snap-poc) re-visit the need of this specific timeout as the algorithm evolves
-        this.timeoutWaitingSnapChunk = Duration.ofSeconds(timeoutWaitingSnapChunk);
         this.snapshotSyncLimit = snapshotSyncLimit;
 
         List<Node> snapBootNodesList = snapBootNodes != null ? snapBootNodes : Collections.emptyList();
@@ -177,10 +171,6 @@ public boolean isClientSnapSyncEnabled() {
         return isClientSnapSyncEnabled;
     }
 
-    public Duration getTimeoutWaitingSnapChunk() {
-        return timeoutWaitingSnapChunk;
-    }
-
     public int getSnapshotSyncLimit() {
         return snapshotSyncLimit;
     }

diff --git a/rskj-core/src/main/resources/expected.conf b/rskj-core/src/main/resources/expected.conf
@@ -281,7 +281,6 @@ sync = {
             enabled = <bool>
             parallel = <bool>
             chunkSize = <chunkSize>
-            chunkRequestTimeout = <chunkRequestTimeout>
             limit = <limit>
             snapBootNodes = [
                 {

diff --git a/rskj-core/src/main/resources/reference.conf b/rskj-core/src/main/resources/reference.conf
@@ -380,8 +380,6 @@ sync {
             enabled = false
             # Server / chunk size
             chunkSize = 50
-            # Request timeout (in seconds)
-            chunkRequestTimeout = 120
             # Distance to the tip of the blockchain to start snapshot sync
             limit = 10000
             # Parallel requests (true, false)

diff --git a/rskj-core/src/test/java/co/rsk/net/SnapshotProcessorTest.java b/rskj-core/src/test/java/co/rsk/net/SnapshotProcessorTest.java
@@ -416,10 +416,8 @@ void givenErrorRLPData_thenOnStateChunkErrorIsCalled() {
 
         underTest.processStateChunkResponse(snapSyncState, peer, responseMessage);
 
-        verify(snapSyncState, times(1)).onNewChunk();
         verify(underTest, times(1)).onStateChunkResponseError(peer, responseMessage);
         verify(peer, times(1)).sendMessage(any(SnapStateChunkRequestMessage.class));
-
     }
 
     private void initializeBlockchainWithAmountOfBlocks(int numberOfBlocks) {

diff --git a/rskj-core/src/test/java/co/rsk/net/sync/SnapSyncStateTest.java b/rskj-core/src/test/java/co/rsk/net/sync/SnapSyncStateTest.java
@@ -19,7 +19,6 @@
 package co.rsk.net.sync;
 
 import co.rsk.core.BlockDifficulty;
-import co.rsk.net.NodeID;
 import co.rsk.net.Peer;
 import co.rsk.net.SnapshotProcessor;
 import co.rsk.net.messages.SnapBlocksResponseMessage;
@@ -33,19 +32,15 @@
 import org.mockito.ArgumentCaptor;
 
 import java.math.BigInteger;
-import java.net.InetAddress;
-import java.net.UnknownHostException;
 import java.time.Duration;
 import java.util.List;
-import java.util.Optional;
 import java.util.PriorityQueue;
 import java.util.Queue;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.greaterThan;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
@@ -98,27 +93,6 @@ void givenOnEnterWasCalledTwice_thenSyncingStartsOnlyOnce() {
         verify(snapshotProcessor, times(1)).startSyncing(underTest);
     }
 
-    @Test
-    void givenOnMessageTimeOutCalled_thenSyncingStops() {
-        //given-when
-        underTest.setRunning();
-        underTest.onMessageTimeOut();
-        //then
-        verify(syncEventsHandler, times(1)).stopSyncing();
-    }
-
-    @Test
-    void givenNewChunk_thenTimerIsReset() {
-        //given
-        underTest.timeElapsed = Duration.ofMinutes(1);
-        assertThat(underTest.timeElapsed, greaterThan(Duration.ZERO));
-
-        // when
-        underTest.onNewChunk();
-        //then
-        assertThat(underTest.timeElapsed, equalTo(Duration.ZERO));
-    }
-
     @Test
     void givenTickIsCalledBeforeTimeout_thenTimerIsUpdated_andNoTimeoutHappens() {
         //given
@@ -132,24 +106,6 @@ void givenTickIsCalledBeforeTimeout_thenTimerIsUpdated_andNoTimeoutHappens() {
         verify(syncEventsHandler, never()).onErrorSyncing(any(), any(), any(), any());
     }
 
-    @Test
-    void givenTickIsCalledAfterTimeout_thenTimerIsUpdated_andTimeoutHappens() throws UnknownHostException {
-        //given
-        Duration elapsedTime = Duration.ofMinutes(1);
-        underTest.timeElapsed = Duration.ZERO;
-        Peer mockedPeer = mock(Peer.class);
-        NodeID nodeID = mock(NodeID.class);
-        when(mockedPeer.getPeerNodeID()).thenReturn(nodeID);
-        when(mockedPeer.getAddress()).thenReturn(InetAddress.getByName("127.0.0.1"));
-        when(peersInformation.getBestSnapPeer()).thenReturn(Optional.of(mockedPeer));
-        underTest.setRunning();
-        // when
-        underTest.tick(elapsedTime);
-        //then
-        assertThat(underTest.timeElapsed, equalTo(elapsedTime));
-        verify(syncEventsHandler, times(1)).stopSyncing();
-    }
-
     @Test
     void givenFinishIsCalled_thenSyncEventHandlerStopsSync() {
         //given-when