[PAN-1062] Specify pending transaction retention period (2 of 2) (Peg…

…aSysEng#1333)
notlesh · May 14, 2019 · 9975195 · 9975195
1 parent 9fe4e98
commit 9975195
Show file tree

Hide file tree

Showing 25 changed files with 130 additions and 69 deletions.
diff --git a/...c/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java b/...c/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java
@@ -76,6 +76,7 @@ public void startNode(final PantheonNode node) {
               .nodePrivateKeyFile(KeyPairUtil.getDefaultKeyFile(node.homeDirectory()))
               .metricsSystem(noOpMetricsSystem)
               .maxPendingTransactions(PendingTransactions.MAX_PENDING_TRANSACTIONS)
+              .pendingTransactionRetentionPeriod(PendingTransactions.DEFAULT_TX_RETENTION_HOURS)
               .rocksdDbConfiguration(
                   new RocksDbConfiguration.Builder().databaseDir(tempDir).build())
               .ethereumWireProtocolConfiguration(EthereumWireProtocolConfiguration.defaultConfig())

diff --git a/...est/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreatorTest.java b/...est/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueBlockCreatorTest.java
@@ -50,14 +50,12 @@
 import tech.pegasys.pantheon.util.bytes.BytesValue;
 
 import java.util.List;
-import java.util.concurrent.TimeUnit;
 
 import com.google.common.collect.Lists;
 import org.junit.Before;
 import org.junit.Test;
 
 public class CliqueBlockCreatorTest {
-  private static final long TRANSACTION_EVICTION_INTERVAL_MS = TimeUnit.MINUTES.toMillis(1);
 
   private final KeyPair proposerKeyPair = KeyPair.generate();
   private final Address proposerAddress = Util.publicKeyToAddress(proposerKeyPair.getPublicKey());
@@ -116,7 +114,10 @@ public void proposerAddressCanBeExtractFromAConstructedBlock() {
             coinbase,
             parent -> extraData.encode(),
             new PendingTransactions(
-                TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem),
+                PendingTransactions.DEFAULT_TX_RETENTION_HOURS,
+                5,
+                TestClock.fixed(),
+                metricsSystem),
             protocolContext,
             protocolSchedule,
             gasLimit -> gasLimit,
@@ -144,7 +145,10 @@ public void insertsValidVoteIntoConstructedBlock() {
             coinbase,
             parent -> extraData.encode(),
             new PendingTransactions(
-                TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem),
+                PendingTransactions.DEFAULT_TX_RETENTION_HOURS,
+                5,
+                TestClock.fixed(),
+                metricsSystem),
             protocolContext,
             protocolSchedule,
             gasLimit -> gasLimit,
@@ -171,7 +175,10 @@ public void insertsNoVoteWhenAuthInValidators() {
             coinbase,
             parent -> extraData.encode(),
             new PendingTransactions(
-                TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem),
+                PendingTransactions.DEFAULT_TX_RETENTION_HOURS,
+                5,
+                TestClock.fixed(),
+                metricsSystem),
             protocolContext,
             protocolSchedule,
             gasLimit -> gasLimit,
@@ -201,7 +208,10 @@ public void insertsNoVoteWhenAtEpoch() {
             coinbase,
             parent -> extraData.encode(),
             new PendingTransactions(
-                TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem),
+                PendingTransactions.DEFAULT_TX_RETENTION_HOURS,
+                5,
+                TestClock.fixed(),
+                metricsSystem),
             protocolContext,
             protocolSchedule,
             gasLimit -> gasLimit,

diff --git a/...st/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutorTest.java b/...st/java/tech/pegasys/pantheon/consensus/clique/blockcreation/CliqueMinerExecutorTest.java
@@ -44,7 +44,6 @@
 import java.util.List;
 import java.util.Random;
 import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
 
 import com.google.common.collect.Lists;
 import io.vertx.core.json.JsonObject;
@@ -55,7 +54,6 @@ public class CliqueMinerExecutorTest {
 
   private static final GenesisConfigOptions GENESIS_CONFIG_OPTIONS =
       GenesisConfigFile.fromConfig(new JsonObject()).getConfigOptions();
-  private static final long TRANSACTION_EVICTION_INTERVAL_MS = TimeUnit.MINUTES.toMillis(1);
   private final KeyPair proposerKeyPair = KeyPair.generate();
   private Address localAddress;
   private final List<Address> validatorList = Lists.newArrayList();
@@ -93,7 +91,10 @@ public void extraDataCreatedOnEpochBlocksContainsValidators() {
             Executors.newSingleThreadExecutor(),
             CliqueProtocolSchedule.create(GENESIS_CONFIG_OPTIONS, proposerKeyPair),
             new PendingTransactions(
-                TRANSACTION_EVICTION_INTERVAL_MS, 1, TestClock.fixed(), metricsSystem),
+                PendingTransactions.DEFAULT_TX_RETENTION_HOURS,
+                1,
+                TestClock.fixed(),
+                metricsSystem),
             proposerKeyPair,
             new MiningParameters(AddressHelpers.ofValue(1), Wei.ZERO, wrappedVanityData, false),
             mock(CliqueBlockScheduler.class),
@@ -124,7 +125,10 @@ public void extraDataForNonEpochBlocksDoesNotContainValidaors() {
             Executors.newSingleThreadExecutor(),
             CliqueProtocolSchedule.create(GENESIS_CONFIG_OPTIONS, proposerKeyPair),
             new PendingTransactions(
-                TRANSACTION_EVICTION_INTERVAL_MS, 1, TestClock.fixed(), metricsSystem),
+                PendingTransactions.DEFAULT_TX_RETENTION_HOURS,
+                1,
+                TestClock.fixed(),
+                metricsSystem),
             proposerKeyPair,
             new MiningParameters(AddressHelpers.ofValue(1), Wei.ZERO, wrappedVanityData, false),
             mock(CliqueBlockScheduler.class),

diff --git a/...ntegration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java b/...ntegration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java
@@ -80,14 +80,12 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
 import com.google.common.collect.Iterables;
 
 public class TestContextBuilder {
 
-  private static final long TRANSACTION_EVICTION_INTERVAL_MS = TimeUnit.MINUTES.toMillis(1);
   private static MetricsSystem metricsSystem = new NoOpMetricsSystem();
 
   private static class ControllerAndState {
@@ -286,7 +284,8 @@ private static ControllerAndState createControllerAndFinalState(
             blockChain, worldStateArchive, new IbftContext(voteTallyCache, voteProposer));
 
     final PendingTransactions pendingTransactions =
-        new PendingTransactions(TRANSACTION_EVICTION_INTERVAL_MS, 1, clock, metricsSystem);
+        new PendingTransactions(
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 1, clock, metricsSystem);
 
     final IbftBlockCreatorFactory blockCreatorFactory =
         new IbftBlockCreatorFactory(

diff --git a/...rc/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java b/...rc/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java
@@ -46,13 +46,11 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
-import java.util.concurrent.TimeUnit;
 
 import com.google.common.collect.Lists;
 import org.junit.Test;
 
 public class IbftBlockCreatorTest {
-  private static final long TRANSACTION_EVICTION_INTERVAL_MS = TimeUnit.MINUTES.toMillis(1);
   private final MetricsSystem metricsSystem = new NoOpMetricsSystem();
 
   @Test
@@ -85,7 +83,7 @@ public void createdBlockPassesValidationRulesAndHasAppropriateHashAndMixHash() {
 
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 1, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 1, TestClock.fixed(), metricsSystem);
 
     final IbftBlockCreator blockCreator =
         new IbftBlockCreator(

diff --git a/...t/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java b/...t/java/tech/pegasys/pantheon/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java
@@ -46,13 +46,11 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
-import java.util.concurrent.TimeUnit;
 
 import com.google.common.collect.Lists;
 import org.junit.Test;
 
 public class IbftBlockCreatorTest {
-  private static final long TRANSACTION_EVICTION_INTERVAL_MS = TimeUnit.MINUTES.toMillis(1);
   private final MetricsSystem metricsSystem = new NoOpMetricsSystem();
 
   @Test
@@ -100,7 +98,10 @@ public void headerProducedPassesValidationRules() {
                         initialValidatorList)
                     .encode(),
             new PendingTransactions(
-                TRANSACTION_EVICTION_INTERVAL_MS, 1, TestClock.fixed(), metricsSystem),
+                PendingTransactions.DEFAULT_TX_RETENTION_HOURS,
+                1,
+                TestClock.fixed(),
+                metricsSystem),
             protContext,
             protocolSchedule,
             parentGasLimit -> parentGasLimit,

diff --git a/.../test/java/tech/pegasys/pantheon/ethereum/blockcreation/BlockTransactionSelectorTest.java b/.../test/java/tech/pegasys/pantheon/ethereum/blockcreation/BlockTransactionSelectorTest.java
@@ -55,7 +55,6 @@
 import java.math.BigInteger;
 import java.time.Instant;
 import java.util.List;
-import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
 
 import com.google.common.collect.Lists;
@@ -64,7 +63,6 @@
 public class BlockTransactionSelectorTest {
 
   private static final KeyPair keyPair = KeyPair.generate();
-  private static final long TRANSACTION_EVICTION_INTERVAL_MS = TimeUnit.MINUTES.toMillis(1);
   private final MetricsSystem metricsSystem = new NoOpMetricsSystem();
 
   @Test
@@ -78,7 +76,7 @@ public void emptyPendingTransactionsResultsInEmptyVettingResult() {
 
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 5, TestClock.fixed(), metricsSystem);
 
     final Supplier<Boolean> isCancelled = () -> false;
 
@@ -118,7 +116,7 @@ public void emptyPendingTransactionsResultsInEmptyVettingResult() {
   public void failedTransactionsAreIncludedInTheBlock() {
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 5, TestClock.fixed(), metricsSystem);
 
     final Transaction transaction = createTransaction(1);
     pendingTransactions.addRemoteTransaction(transaction);
@@ -171,7 +169,7 @@ public void failedTransactionsAreIncludedInTheBlock() {
   public void invalidTransactionsTransactionProcessingAreSkippedButBlockStillFills() {
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 5, TestClock.fixed(), metricsSystem);
 
     final List<Transaction> transactionsToInject = Lists.newArrayList();
     for (int i = 0; i < 5; i++) {
@@ -235,7 +233,7 @@ public void invalidTransactionsTransactionProcessingAreSkippedButBlockStillFills
   public void subsetOfPendingTransactionsIncludedWhenBlockGasLimitHit() {
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 5, TestClock.fixed(), metricsSystem);
 
     final List<Transaction> transactionsToInject = Lists.newArrayList();
     // Transactions are reported in reverse order.
@@ -302,7 +300,7 @@ public void subsetOfPendingTransactionsIncludedWhenBlockGasLimitHit() {
   public void transactionOfferingGasPriceLessThanMinimumIsIdentifiedAndRemovedFromPending() {
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 5, TestClock.fixed(), metricsSystem);
 
     final Blockchain blockchain = new TestBlockchain();
 
@@ -348,7 +346,7 @@ public void transactionOfferingGasPriceLessThanMinimumIsIdentifiedAndRemovedFrom
   public void transactionTooLargeForBlockDoesNotPreventMoreBeingAddedIfBlockOccupancyNotReached() {
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 5, TestClock.fixed(), metricsSystem);
 
     final Blockchain blockchain = new TestBlockchain();
     final DefaultMutableWorldState worldState = inMemoryWorldState();
@@ -421,7 +419,7 @@ public void transactionTooLargeForBlockDoesNotPreventMoreBeingAddedIfBlockOccupa
   public void transactionSelectionStopsWhenSufficientBlockOccupancyIsReached() {
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 5, TestClock.fixed(), metricsSystem);
 
     final Blockchain blockchain = new TestBlockchain();
     final DefaultMutableWorldState worldState = inMemoryWorldState();
@@ -505,7 +503,7 @@ public void transactionSelectionStopsWhenSufficientBlockOccupancyIsReached() {
   public void shouldDiscardTransactionsThatFailValidation() {
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 5, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 5, TestClock.fixed(), metricsSystem);
 
     final TransactionProcessor transactionProcessor = mock(TransactionProcessor.class);
     final Blockchain blockchain = new TestBlockchain();

diff --git a/...n/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashBlockCreatorTest.java b/...n/src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashBlockCreatorTest.java
@@ -30,7 +30,6 @@
 
 import java.io.IOException;
 import java.math.BigInteger;
-import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 
 import com.google.common.collect.Lists;
@@ -48,7 +47,6 @@ public class EthHashBlockCreatorTest {
 
   private static final BytesValue BLOCK_1_EXTRA_DATA =
       BytesValue.fromHexString("0x476574682f76312e302e302f6c696e75782f676f312e342e32");
-  private static final long TRANSACTION_EVICTION_INTERVAL_MS = TimeUnit.MINUTES.toMillis(1);
   private final MetricsSystem metricsSystem = new NoOpMetricsSystem();
 
   private final ExecutionContextTestFixture executionContextTestFixture =
@@ -68,7 +66,7 @@ public void createMainnetBlock1() throws IOException {
 
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 1, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 1, TestClock.fixed(), metricsSystem);
 
     final EthHashBlockCreator blockCreator =
         new EthHashBlockCreator(

diff --git a/.../src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashMinerExecutorTest.java b/.../src/test/java/tech/pegasys/pantheon/ethereum/blockcreation/EthHashMinerExecutorTest.java
@@ -23,12 +23,10 @@
 import tech.pegasys.pantheon.util.Subscribers;
 
 import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
 
 import org.junit.Test;
 
 public class EthHashMinerExecutorTest {
-  private static final long TRANSACTION_EVICTION_INTERVAL_MS = TimeUnit.MINUTES.toMillis(1);
   private final MetricsSystem metricsSystem = new NoOpMetricsSystem();
 
   @Test
@@ -38,7 +36,7 @@ public void startingMiningWithoutCoinbaseThrowsException() {
 
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 1, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 1, TestClock.fixed(), metricsSystem);
 
     final EthHashMinerExecutor executor =
         new EthHashMinerExecutor(
@@ -60,7 +58,7 @@ public void settingCoinbaseToNullThrowsException() {
 
     final PendingTransactions pendingTransactions =
         new PendingTransactions(
-            TRANSACTION_EVICTION_INTERVAL_MS, 1, TestClock.fixed(), metricsSystem);
+            PendingTransactions.DEFAULT_TX_RETENTION_HOURS, 1, TestClock.fixed(), metricsSystem);
 
     final EthHashMinerExecutor executor =
         new EthHashMinerExecutor(

diff --git a/...th/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/PendingTransactions.java b/...th/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/PendingTransactions.java
@@ -27,6 +27,7 @@
 
 import java.time.Clock;
 import java.time.Instant;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -49,6 +50,10 @@
  */
 public class PendingTransactions {
   public static final int MAX_PENDING_TRANSACTIONS = 4096;
+  public static final int DEFAULT_TX_RETENTION_HOURS = 13;
+
+  private final int maxTransactionRetentionHours;
+  private final Clock clock;
 
   private final Map<Hash, TransactionInfo> pendingTransactions = new HashMap<>();
   private final SortedSet<TransactionInfo> prioritizedTransactions =
@@ -64,21 +69,18 @@ public class PendingTransactions {
   private final Subscribers<PendingTransactionDroppedListener> transactionDroppedListeners =
       new Subscribers<>();
 
-  private final int maxPendingTransactions;
-  private final Clock clock;
-
   private final LabelledMetric<Counter> transactionRemovedCounter;
   private final Counter localTransactionAddedCounter;
   private final Counter remoteTransactionAddedCounter;
 
-  private final long transactionEvictionIntervalMs;
+  private final long maxPendingTransactions;
 
   public PendingTransactions(
-      final long transactionEvictionIntervalMs,
+      final int maxTransactionRetentionHours,
       final int maxPendingTransactions,
       final Clock clock,
       final MetricsSystem metricsSystem) {
-    this.transactionEvictionIntervalMs = transactionEvictionIntervalMs;
+    this.maxTransactionRetentionHours = maxTransactionRetentionHours;
     this.maxPendingTransactions = maxPendingTransactions;
     this.clock = clock;
     final LabelledMetric<Counter> transactionAddedCounter =
@@ -102,7 +104,7 @@ public PendingTransactions(
   public void evictOldTransactions() {
     synchronized (pendingTransactions) {
       final Instant removeTransactionsBefore =
-          clock.instant().minusMillis(transactionEvictionIntervalMs);
+          clock.instant().minus(maxTransactionRetentionHours, ChronoUnit.HOURS);
       final List<TransactionInfo> transactionsToRemove =
           prioritizedTransactions.stream()
               .filter(