From b1f44fc9a7caa7f4866e20920c02d614eeff0b02 Mon Sep 17 00:00:00 2001 From: tmohay Date: Fri, 14 Dec 2018 16:10:30 +1100 Subject: [PATCH 1/6] Update IbftConfig Fields The RequestTimeout field in the IbftConfig has been updated to be "RequestTimeoutSeconds". --- .../tech/pegasys/pantheon/config/IbftConfigOptions.java | 4 ++-- .../pegasys/pantheon/config/IbftConfigOptionsTest.java | 8 ++++---- .../pantheon/controller/IbftPantheonController.java | 2 +- pantheon/src/test/resources/ibftlegacy_genesis.json | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java b/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java index bf1febc117..819ac64e15 100644 --- a/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java +++ b/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java @@ -36,7 +36,7 @@ public int getBlockPeriodSeconds() { return ibftConfigRoot.getInteger("blockperiodseconds", DEFAULT_BLOCK_PERIOD_SECONDS); } - public int getRequestTimeoutMillis() { - return ibftConfigRoot.getInteger("requesttimeout", DEFAULT_ROUND_EXPIRY_MILLISECONDS); + public int getRequestTimeoutSeconds() { + return ibftConfigRoot.getInteger("requesttimeoutseconds", DEFAULT_ROUND_EXPIRY_MILLISECONDS); } } diff --git a/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java b/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java index 9ed52d2356..22703c7b45 100644 --- a/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java +++ b/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java @@ -64,19 +64,19 @@ public void shouldGetDefaultBlockPeriodFromDefaultConfig() { @Test public void shouldGetRequestTimeoutFromConfig() { - final IbftConfigOptions config = fromConfigOptions(singletonMap("RequestTimeout", 5)); - assertThat(config.getRequestTimeoutMillis()).isEqualTo(5); + final IbftConfigOptions config = fromConfigOptions(singletonMap("RequestTimeoutSeconds", 5)); + assertThat(config.getRequestTimeoutSeconds()).isEqualTo(5); } @Test public void shouldFallbackToDefaultRequestTimeout() { final IbftConfigOptions config = fromConfigOptions(emptyMap()); - assertThat(config.getRequestTimeoutMillis()).isEqualTo(EXPECTED_DEFAULT_REQUEST_TIMEOUT); + assertThat(config.getRequestTimeoutSeconds()).isEqualTo(EXPECTED_DEFAULT_REQUEST_TIMEOUT); } @Test public void shouldGetDefaultRequestTimeoutFromDefaultConfig() { - assertThat(IbftConfigOptions.DEFAULT.getRequestTimeoutMillis()) + assertThat(IbftConfigOptions.DEFAULT.getRequestTimeoutSeconds()) .isEqualTo(EXPECTED_DEFAULT_REQUEST_TIMEOUT); } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java index 42597ec8fe..693746d7ff 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java @@ -187,7 +187,7 @@ public static PantheonController init( final IbftStateMachine ibftStateMachine = new IbftStateMachine(blockCreatorFactory); final IbftProcessor ibftProcessor = - new IbftProcessor(ibftEventQueue, ibftConfig.getRequestTimeoutMillis(), ibftStateMachine); + new IbftProcessor(ibftEventQueue, ibftConfig.getRequestTimeoutSeconds(), ibftStateMachine); final ExecutorService processorExecutor = Executors.newSingleThreadExecutor(); processorExecutor.submit(ibftProcessor); diff --git a/pantheon/src/test/resources/ibftlegacy_genesis.json b/pantheon/src/test/resources/ibftlegacy_genesis.json index 870a49f48e..84a297fd19 100644 --- a/pantheon/src/test/resources/ibftlegacy_genesis.json +++ b/pantheon/src/test/resources/ibftlegacy_genesis.json @@ -10,7 +10,7 @@ "ibft": { "epochLength": 30000, "blockPeriodSeconds" : 1, - "requestTimeout": 10000 + "requestTimeoutSeconds": 10 } }, "nonce": "0x0", From 9ad38958f1831bf0c7e2a98eabe10315bc9f976e Mon Sep 17 00:00:00 2001 From: tmohay Date: Fri, 14 Dec 2018 16:44:59 +1100 Subject: [PATCH 2/6] repaired default value --- .../java/tech/pegasys/pantheon/config/IbftConfigOptions.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java b/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java index 819ac64e15..4bb943c965 100644 --- a/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java +++ b/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java @@ -20,7 +20,7 @@ public class IbftConfigOptions { private static final long DEFAULT_EPOCH_LENGTH = 30_000; private static final int DEFAULT_BLOCK_PERIOD_SECONDS = 1; - private static final int DEFAULT_ROUND_EXPIRY_MILLISECONDS = 10000; + private static final int DEFAULT_ROUND_EXPIRY_SECONDS = 1; private final JsonObject ibftConfigRoot; @@ -37,6 +37,6 @@ public int getBlockPeriodSeconds() { } public int getRequestTimeoutSeconds() { - return ibftConfigRoot.getInteger("requesttimeoutseconds", DEFAULT_ROUND_EXPIRY_MILLISECONDS); + return ibftConfigRoot.getInteger("requesttimeoutseconds", DEFAULT_ROUND_EXPIRY_SECONDS); } } From 70f28ba9fba918b15f539282b9dfe06f5d72f428 Mon Sep 17 00:00:00 2001 From: tmohay Date: Fri, 14 Dec 2018 16:49:09 +1100 Subject: [PATCH 3/6] Reworked RoundTimer to take seconds, but use ms internally --- .../consensus/ibft/IbftProcessor.java | 20 ++++++++++++------- .../pantheon/consensus/ibft/RoundTimer.java | 6 +++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProcessor.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProcessor.java index 11b04d08f5..e55bbf6c5e 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProcessor.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProcessor.java @@ -21,8 +21,11 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -/** Execution context for draining queued ibft events and applying them to a maintained state */ +/** + * Execution context for draining queued ibft events and applying them to a maintained state + */ public class IbftProcessor implements Runnable { + private static final Logger LOG = LogManager.getLogger(); private final IbftEventQueue incomingQueue; @@ -35,18 +38,18 @@ public class IbftProcessor implements Runnable { * Construct a new IbftProcessor * * @param incomingQueue The event queue from which to drain new events - * @param baseRoundExpiryMillis The expiry time in milliseconds of round 0 + * @param baseRoundExpirySeconds The expiry time in milliseconds of round 0 * @param stateMachine an IbftStateMachine ready to process events and maintain state */ public IbftProcessor( final IbftEventQueue incomingQueue, - final int baseRoundExpiryMillis, + final int baseRoundExpirySeconds, final IbftStateMachine stateMachine) { // Spawning the round timer with a single thread as we should never have more than 1 timer in // flight at a time this( incomingQueue, - baseRoundExpiryMillis, + baseRoundExpirySeconds, stateMachine, Executors.newSingleThreadScheduledExecutor()); } @@ -54,17 +57,20 @@ public IbftProcessor( @VisibleForTesting IbftProcessor( final IbftEventQueue incomingQueue, - final int baseRoundExpiryMillis, + final int baseRoundExpirySeconds, final IbftStateMachine stateMachine, final ScheduledExecutorService roundTimerExecutor) { this.incomingQueue = incomingQueue; this.roundTimerExecutor = roundTimerExecutor; - this.roundTimer = new RoundTimer(incomingQueue, baseRoundExpiryMillis, roundTimerExecutor); + this.roundTimer = + new RoundTimer(incomingQueue, baseRoundExpirySeconds * 1000, roundTimerExecutor); this.stateMachine = stateMachine; } - /** Indicate to the processor that it should gracefully stop at its next opportunity */ + /** + * Indicate to the processor that it should gracefully stop at its next opportunity + */ public void stop() { shutdown = true; } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/RoundTimer.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/RoundTimer.java index 970299296c..50f6a74543 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/RoundTimer.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/RoundTimer.java @@ -30,17 +30,17 @@ public class RoundTimer { * Construct a RoundTimer with primed executor service ready to start timers * * @param queue The queue in which to put round expiry events - * @param baseExpiryMillis The initial round length for round 0 + * @param baseExpirySeconds The initial round length for round 0 * @param timerExecutor executor service that timers can be scheduled with */ public RoundTimer( final IbftEventQueue queue, - final long baseExpiryMillis, + final long baseExpirySeconds, final ScheduledExecutorService timerExecutor) { this.queue = queue; this.timerExecutor = timerExecutor; this.currentTimerTask = Optional.empty(); - this.baseExpiryMillis = baseExpiryMillis; + this.baseExpiryMillis = baseExpirySeconds * 1000; } /** Cancels the current running round timer if there is one */ From abef7e722d3608772aca680230bbc35b815cedd5 Mon Sep 17 00:00:00 2001 From: tmohay Date: Fri, 14 Dec 2018 16:57:46 +1100 Subject: [PATCH 4/6] repaired tests --- .../pegasys/pantheon/config/IbftConfigOptionsTest.java | 2 +- .../pegasys/pantheon/consensus/ibft/IbftProcessor.java | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java b/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java index 22703c7b45..18add31e3a 100644 --- a/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java +++ b/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java @@ -25,7 +25,7 @@ public class IbftConfigOptionsTest { private static final int EXPECTED_DEFAULT_EPOCH_LENGTH = 30_000; private static final int EXPECTED_DEFAULT_BLOCK_PERIOD = 1; - private static final int EXPECTED_DEFAULT_REQUEST_TIMEOUT = 10_000; + private static final int EXPECTED_DEFAULT_REQUEST_TIMEOUT = 1; @Test public void shouldGetEpochLengthFromConfig() { diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProcessor.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProcessor.java index e55bbf6c5e..70944906cb 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProcessor.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProcessor.java @@ -21,9 +21,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -/** - * Execution context for draining queued ibft events and applying them to a maintained state - */ +/** Execution context for draining queued ibft events and applying them to a maintained state */ public class IbftProcessor implements Runnable { private static final Logger LOG = LogManager.getLogger(); @@ -68,9 +66,7 @@ public IbftProcessor( this.stateMachine = stateMachine; } - /** - * Indicate to the processor that it should gracefully stop at its next opportunity - */ + /** Indicate to the processor that it should gracefully stop at its next opportunity */ public void stop() { shutdown = true; } From 3edea6ded3849d849555032637efb393ec5c85e9 Mon Sep 17 00:00:00 2001 From: tmohay Date: Sat, 15 Dec 2018 09:59:18 +1100 Subject: [PATCH 5/6] Empty to kick jenkins From acd95637b5825e8abd2622099ad5fe4e783df4b5 Mon Sep 17 00:00:00 2001 From: tmohay Date: Sat, 15 Dec 2018 21:54:35 +1100 Subject: [PATCH 6/6] Repair Roundtimer tests --- .../pegasys/pantheon/consensus/ibft/RoundTimerTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/RoundTimerTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/RoundTimerTest.java index 9b31a82681..dcf75f91dd 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/RoundTimerTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/RoundTimerTest.java @@ -59,22 +59,22 @@ public void cancelTimerCancelsWhenNoTimer() { @Test public void startTimerSchedulesTimerCorrectlyForRound0() { - checkTimerForRound(0, 1); + checkTimerForRound(0, 1000); } @Test public void startTimerSchedulesTimerCorrectlyForRound1() { - checkTimerForRound(1, 2); + checkTimerForRound(1, 2000); } @Test public void startTimerSchedulesTimerCorrectlyForRound2() { - checkTimerForRound(2, 4); + checkTimerForRound(2, 4000); } @Test public void startTimerSchedulesTimerCorrectlyForRound3() { - checkTimerForRound(3, 8); + checkTimerForRound(3, 8000); } private void checkTimerForRound(final int roundNumber, final long timeout) {