Merge branch 'refs/heads/7311-add-peertask-foundation-code' into 7311-add-GetReceiptsFromPeerTask

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTask.java

@@ -75,9 +75,9 @@ public interface PeerTask<T> {
   Predicate<EthPeer> getPeerRequirementFilter();
 
   /**
-   * Checks if the supplied result is considered a partial success
+   * Checks if the supplied result is considered a success
    *
-   * @return true if the supplied result is considered a partial success
+   * @return true if the supplied result is considered a success
    */
-  boolean isPartialSuccess(T result);
+  boolean isSuccess(T result);
 }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutor.java

@@ -40,7 +40,6 @@ public class PeerTaskExecutor {
   private final PeerTaskRequestSender requestSender;
 
   private final LabelledMetric<OperationTimer> requestTimer;
-  private final LabelledMetric<Counter> partialSuccessCounter;
   private final LabelledMetric<Counter> timeoutCounter;
   private final LabelledMetric<Counter> invalidResponseCounter;
   private final LabelledMetric<Counter> internalExceptionCounter;
@@ -59,12 +58,6 @@ public class PeerTaskExecutor {
             "request_time",
             "Time taken to send a request and receive a response",
             "taskName");
-    partialSuccessCounter =
-        metricsSystem.createLabelledCounter(
-            BesuMetricCategory.PEERS,
-            "partial_success_total",
-            "Counter of the number of partial success occurred",
-            "taskName");
     timeoutCounter =
         metricsSystem.createLabelledCounter(
             BesuMetricCategory.PEERS,
@@ -145,16 +138,18 @@ public class PeerTaskExecutor {
           inflightRequestCountForThisTaskClass.decrementAndGet();
         }
 
-        if (peerTask.isPartialSuccess(result)) {
+        if (peerTask.isSuccess(result)) {
-          partialSuccessCounter.labels(taskClassName).inc();
+          peer.recordUsefulResponse();
           executorResult =
               new PeerTaskExecutorResult<>(
-                  Optional.ofNullable(result), PeerTaskExecutorResponseCode.PARTIAL_SUCCESS);
+                  Optional.ofNullable(result), PeerTaskExecutorResponseCode.SUCCESS);
         } else {
-          peer.recordUsefulResponse();
+          // At this point, the result is most likely empty. Technically, this is a valid result, so
+          // we don't penalise the peer, but it's also a useless result, so we return
+          // INVALID_RESPONSE code
           executorResult =
               new PeerTaskExecutorResult<>(
-                  Optional.ofNullable(result), PeerTaskExecutorResponseCode.SUCCESS);
+                  Optional.ofNullable(result), PeerTaskExecutorResponseCode.INVALID_RESPONSE);
         }
 
       } catch (PeerNotConnected e) {

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorResponseCode.java

@@ -16,7 +16,6 @@ package org.hyperledger.besu.ethereum.eth.manager.peertask;
 
 public enum PeerTaskExecutorResponseCode {
   SUCCESS,
-  PARTIAL_SUCCESS,
   NO_PEER_AVAILABLE,
   PEER_DISCONNECTED,
   INTERNAL_SERVER_ERROR,

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/peertask/PeerTaskExecutorTest.java

@@ -73,7 +73,7 @@ public class PeerTaskExecutorTest {
     Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer))
         .thenReturn(responseMessageData);
     Mockito.when(peerTask.parseResponse(responseMessageData)).thenReturn(responseObject);
-    Mockito.when(peerTask.isPartialSuccess(responseObject)).thenReturn(false);
+    Mockito.when(peerTask.isSuccess(responseObject)).thenReturn(true);
 
     PeerTaskExecutorResult<Object> result = peerTaskExecutor.executeAgainstPeer(peerTask, ethPeer);
 
@@ -102,14 +102,13 @@ public class PeerTaskExecutorTest {
     Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer))
         .thenReturn(responseMessageData);
     Mockito.when(peerTask.parseResponse(responseMessageData)).thenReturn(responseObject);
-    Mockito.when(peerTask.isPartialSuccess(responseObject)).thenReturn(true);
+    Mockito.when(peerTask.isSuccess(responseObject)).thenReturn(false);
 
     PeerTaskExecutorResult<Object> result = peerTaskExecutor.executeAgainstPeer(peerTask, ethPeer);
 
     Assertions.assertNotNull(result);
-    Assertions.assertTrue(result.result().isPresent());
+    Assertions.assertTrue(result.result().isEmpty());
-    Assertions.assertSame(responseObject, result.result().get());
+    Assertions.assertEquals(PeerTaskExecutorResponseCode.INVALID_RESPONSE, result.responseCode());
-    Assertions.assertEquals(PeerTaskExecutorResponseCode.PARTIAL_SUCCESS, result.responseCode());
   }
 
   @Test
@@ -132,7 +131,7 @@ public class PeerTaskExecutorTest {
         .thenReturn(responseMessageData);
     Mockito.when(requestMessageData.getCode()).thenReturn(requestMessageDataCode);
     Mockito.when(peerTask.parseResponse(responseMessageData)).thenReturn(responseObject);
-    Mockito.when(peerTask.isPartialSuccess(responseObject)).thenReturn(false);
+    Mockito.when(peerTask.isSuccess(responseObject)).thenReturn(true);
 
     PeerTaskExecutorResult<Object> result = peerTaskExecutor.executeAgainstPeer(peerTask, ethPeer);
 
@@ -238,7 +237,7 @@ public class PeerTaskExecutorTest {
     Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, ethPeer))
         .thenReturn(responseMessageData);
     Mockito.when(peerTask.parseResponse(responseMessageData)).thenReturn(responseObject);
-    Mockito.when(peerTask.isPartialSuccess(responseObject)).thenReturn(false);
+    Mockito.when(peerTask.isSuccess(responseObject)).thenReturn(true);
 
     PeerTaskExecutorResult<Object> result = peerTaskExecutor.executeAgainstPeer(peerTask, ethPeer);
 
@@ -276,7 +275,7 @@ public class PeerTaskExecutorTest {
     Mockito.when(requestSender.sendRequest(subprotocol, requestMessageData, peer2))
         .thenReturn(responseMessageData);
     Mockito.when(peerTask.parseResponse(responseMessageData)).thenReturn(responseObject);
-    Mockito.when(peerTask.isPartialSuccess(responseObject)).thenReturn(false);
+    Mockito.when(peerTask.isSuccess(responseObject)).thenReturn(true);
 
     PeerTaskExecutorResult<Object> result = peerTaskExecutor.execute(peerTask);