Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Update IbftConfig Fields #422

Merged
merged 8 commits into from
Dec 16, 2018
+24 −22
Merged

Update IbftConfig Fields #422

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b1f44fc
Update IbftConfig Fields
Dec 14, 2018
9ad3895
repaired default value
Dec 14, 2018
eec894d
Merge remote-tracking branch 'upstream/master' into ibft_config_seconds
Dec 14, 2018
70f28ba
Reworked RoundTimer to take seconds, but use ms internally
Dec 14, 2018
abef7e7
repaired tests
Dec 14, 2018
a4658cd
Merge remote-tracking branch 'upstream/master' into ibft_config_seconds
Dec 14, 2018
3edea6d
Empty to kick jenkins
Dec 14, 2018
acd9563
Repair Roundtimer tests
Dec 15, 2018
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
4 changes: 2 additions & 2 deletions config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the name requestTimeout doesn't really indicate what it is for. Some mention of the round in name would be good. Perhaps something like initialRoundTimeoutSeconds

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but can we worry about this one next, don't want to stray TOO far right now.

return ibftConfigRoot.getInteger("requesttimeoutseconds", DEFAULT_ROUND_EXPIRY_MILLISECONDS);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still got DEFAULT_ROUND_EXPIRY_MILLISECONDS while we're now talking about seconds

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops. Done.

}
}
8 changes: 4 additions & 4 deletions config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ public static PantheonController<IbftContext> init(

final IbftStateMachine ibftStateMachine = new IbftStateMachine(blockCreatorFactory);
final IbftProcessor ibftProcessor =
new IbftProcessor(ibftEventQueue, ibftConfig.getRequestTimeoutMillis(), ibftStateMachine);
new IbftProcessor(ibftEventQueue, ibftConfig.getRequestTimeoutSeconds(), ibftStateMachine);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Far side of this call is still referring to it's parameter as millis

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

final ExecutorService processorExecutor = Executors.newSingleThreadExecutor();
processorExecutor.submit(ibftProcessor);

Expand Down
2 changes: 1 addition & 1 deletion pantheon/src/test/resources/ibftlegacy_genesis.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"ibft": {
"epochLength": 30000,
"blockPeriodSeconds" : 1,
"requestTimeout": 10000
"requestTimeoutSeconds": 10
}
},
"nonce": "0x0",
Expand Down