Skip to content

Commit

Permalink
Switch to new sync target if it exceeds the td threshold (PegaSysEng#…
Browse files Browse the repository at this point in the history
…1286)

Increase total difficulty threshold required to switch sync targets.
Increase threshold for changing sync target based on height to 200.
  • Loading branch information
ajsutton authored and notlesh committed May 4, 2019
1 parent c6c2179 commit 18d5896
Show file tree
Hide file tree
Showing 5 changed files with 232 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -244,20 +244,20 @@ public void parseBlockPropagationRange(final String arg) {
@CommandLine.Option(
names = "--Xsynchronizer-downloader-change-target-threshold-by-height",
hidden = true,
defaultValue = "20",
defaultValue = "200",
paramLabel = "<LONG>",
description =
"Minimum height difference before switching fast sync download peers (default: ${DEFAULT-VALUE})")
private long downloaderChangeTargetThresholdByHeight = 20L;
private long downloaderChangeTargetThresholdByHeight = 200L;

@CommandLine.Option(
names = "--Xsynchronizer-downloader-change-target-threshold-by-td",
hidden = true,
defaultValue = "1000000000",
defaultValue = "1000000000000000000",
paramLabel = "<UINT256>",
description =
"Minimum total difficulty difference before switching fast sync download peers (default: ${DEFAULT-VALUE})")
private UInt256 downloaderChangeTargetThresholdByTd = UInt256.of(1_000_000_000L);
private UInt256 downloaderChangeTargetThresholdByTd = UInt256.of(1_000_000_000_000_000_000L);

@CommandLine.Option(
names = "--Xsynchronizer-downloader-header-request-size",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright 2019 ConsenSys AG.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package tech.pegasys.pantheon.ethereum.eth.sync.fullsync;

import tech.pegasys.pantheon.ethereum.eth.manager.ChainState;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeer;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeers;
import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration;
import tech.pegasys.pantheon.util.uint.UInt256;

import java.util.Optional;

public class BetterSyncTargetEvaluator {

private final SynchronizerConfiguration config;
private final EthPeers ethPeers;

public BetterSyncTargetEvaluator(
final SynchronizerConfiguration config, final EthPeers ethPeers) {
this.config = config;
this.ethPeers = ethPeers;
}

public boolean shouldSwitchSyncTarget(final EthPeer currentSyncTarget) {
final ChainState currentPeerChainState = currentSyncTarget.chainState();
final Optional<EthPeer> maybeBestPeer = ethPeers.bestPeer();

return maybeBestPeer
.map(
bestPeer -> {
if (EthPeers.BEST_CHAIN.compare(bestPeer, currentSyncTarget) <= 0) {
// Our current target is better or equal to the best peer
return false;
}
// Require some threshold to be exceeded before switching targets to keep some
// stability when multiple peers are in range of each other
final ChainState bestPeerChainState = bestPeer.chainState();
final UInt256 tdDifference =
bestPeerChainState
.getBestBlock()
.getTotalDifficulty()
.minus(currentPeerChainState.getBestBlock().getTotalDifficulty());
if (tdDifference.compareTo(config.downloaderChangeTargetThresholdByTd()) > 0) {
return true;
}
final long heightDifference =
bestPeerChainState.getEstimatedHeight()
- currentPeerChainState.getEstimatedHeight();
return heightDifference > config.downloaderChangeTargetThresholdByHeight();
})
.orElse(false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@
import tech.pegasys.pantheon.ethereum.ProtocolContext;
import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain;
import tech.pegasys.pantheon.ethereum.core.BlockHeader;
import tech.pegasys.pantheon.ethereum.eth.manager.ChainState;
import tech.pegasys.pantheon.ethereum.eth.manager.EthContext;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeer;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeers;
import tech.pegasys.pantheon.ethereum.eth.sync.SyncTargetManager;
import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration;
import tech.pegasys.pantheon.ethereum.eth.sync.state.SyncTarget;
Expand All @@ -38,9 +36,9 @@
class FullSyncTargetManager<C> extends SyncTargetManager<C> {

private static final Logger LOG = LogManager.getLogger();
private final SynchronizerConfiguration config;
private final ProtocolContext<C> protocolContext;
private final EthContext ethContext;
private final BetterSyncTargetEvaluator betterSyncTargetEvaluator;

FullSyncTargetManager(
final SynchronizerConfiguration config,
Expand All @@ -49,9 +47,9 @@ class FullSyncTargetManager<C> extends SyncTargetManager<C> {
final EthContext ethContext,
final MetricsSystem metricsSystem) {
super(config, protocolSchedule, protocolContext, ethContext, metricsSystem);
this.config = config;
this.protocolContext = protocolContext;
this.ethContext = ethContext;
betterSyncTargetEvaluator = new BetterSyncTargetEvaluator(config, ethContext.getEthPeers());
}

@Override
Expand Down Expand Up @@ -100,36 +98,7 @@ public boolean isSyncTargetReached(final EthPeer peer) {

@Override
public boolean shouldSwitchSyncTarget(final SyncTarget currentTarget) {
final EthPeer currentPeer = currentTarget.peer();
final ChainState currentPeerChainState = currentPeer.chainState();
final Optional<EthPeer> maybeBestPeer = ethContext.getEthPeers().bestPeer();

return maybeBestPeer
.map(
bestPeer -> {
if (EthPeers.BEST_CHAIN.compare(bestPeer, currentPeer) <= 0) {
// Our current target is better or equal to the best peer
return false;
}
// Require some threshold to be exceeded before switching targets to keep some
// stability
// when multiple peers are in range of each other
final ChainState bestPeerChainState = bestPeer.chainState();
final long heightDifference =
bestPeerChainState.getEstimatedHeight()
- currentPeerChainState.getEstimatedHeight();
if (heightDifference == 0 && bestPeerChainState.getEstimatedHeight() == 0) {
// Only check td if we don't have a height metric
final UInt256 tdDifference =
bestPeerChainState
.getBestBlock()
.getTotalDifficulty()
.minus(currentPeerChainState.getBestBlock().getTotalDifficulty());
return tdDifference.compareTo(config.downloaderChangeTargetThresholdByTd()) > 0;
}
return heightDifference > config.downloaderChangeTargetThresholdByHeight();
})
.orElse(false);
return betterSyncTargetEvaluator.shouldSwitchSyncTarget(currentTarget.peer());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
/*
* Copyright 2019 ConsenSys AG.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package tech.pegasys.pantheon.ethereum.eth.sync.fullsync;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import tech.pegasys.pantheon.ethereum.core.Hash;
import tech.pegasys.pantheon.ethereum.eth.manager.ChainState;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeer;
import tech.pegasys.pantheon.ethereum.eth.manager.EthPeers;
import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration;
import tech.pegasys.pantheon.util.uint.UInt256;

import java.util.Optional;

import org.junit.Test;

public class BetterSyncTargetEvaluatorTest {

private static final int CURRENT_TARGET_HEIGHT = 10;
private static final int CURRENT_TARGET_TD = 50;
private static final int HEIGHT_THRESHOLD = 100;
private static final int TD_THRESHOLD = 5;
private final EthPeers ethPeers = mock(EthPeers.class);
private final EthPeer currentTarget = peer(CURRENT_TARGET_HEIGHT, CURRENT_TARGET_TD);
private final BetterSyncTargetEvaluator evaluator =
new BetterSyncTargetEvaluator(
SynchronizerConfiguration.builder()
.downloaderChangeTargetThresholdByHeight(HEIGHT_THRESHOLD)
.downloaderChangeTargetThresholdByTd(UInt256.of(TD_THRESHOLD))
.build(),
ethPeers);

@Test
public void shouldNotSwitchTargetsIfNoBestPeerIsAvailable() {
when(ethPeers.bestPeer()).thenReturn(Optional.empty());

assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}

@Test
public void shouldNotSwitchTargetWhenBestPeerHasLowerHeightAndDifficulty() {
bestPeerWithDelta(-1, -1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}

@Test
public void shouldNotSwitchTargetWhenBestPeerHasSameHeightAndLowerDifficulty() {
bestPeerWithDelta(0, -1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}

@Test
public void shouldNotSwitchTargetWhenBestPeerHasLowerHeightAndSameDifficulty() {
bestPeerWithDelta(-1, 0);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}

@Test
public void shouldNotSwitchTargetWhenBestPeerHasGreaterHeightAndLowerDifficulty() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, -1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}

@Test
public void shouldNotSwitchTargetWhenBestPeerHasEqualHeightAndDifficulty() {
bestPeerWithDelta(0, 0);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}

@Test
public void shouldNotSwitchWhenHeightAndTdHigherWithinThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD - 1, TD_THRESHOLD - 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}

@Test
public void shouldNotSwitchWhenHeightAndTdHigherEqualToThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD, TD_THRESHOLD);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}

@Test
public void shouldSwitchWhenHeightExceedsThresholdAndDifficultyEqual() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, 0);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}

@Test
public void shouldSwitchWhenHeightExceedsThresholdAndDifficultyWithinThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, TD_THRESHOLD - 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}

@Test
public void shouldSwitchWhenHeightAndDifficultyExceedThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, TD_THRESHOLD + 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}

@Test
public void shouldNotSwitchWhenHeightExceedsThresholdButDifficultyIsLower() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, -1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isFalse();
}

@Test
public void shouldSwitchWhenDifficultyExceedsThresholdAndHeightIsEqual() {
bestPeerWithDelta(0, TD_THRESHOLD + 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}

@Test
public void shouldSwitchWhenDifficultyExceedsThresholdAndHeightIsLower() {
bestPeerWithDelta(-1, TD_THRESHOLD + 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}

@Test
public void shouldSwitchWhenDifficultyExceedsThresholdAndHeightIsWithinThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD - 1, TD_THRESHOLD + 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}

@Test
public void shouldSwitchWhenHeightAndDifficultyExceedsThreshold() {
bestPeerWithDelta(HEIGHT_THRESHOLD + 1, TD_THRESHOLD + 1);
assertThat(evaluator.shouldSwitchSyncTarget(currentTarget)).isTrue();
}

private void bestPeerWithDelta(final long height, final long totalDifficulty) {
final EthPeer bestPeer =
peer(CURRENT_TARGET_HEIGHT + height, CURRENT_TARGET_TD + totalDifficulty);
when(ethPeers.bestPeer()).thenReturn(Optional.of(bestPeer));
}

private EthPeer peer(final long chainHeight, final long totalDifficulty) {
final EthPeer peer = mock(EthPeer.class);
final ChainState chainState = new ChainState();
chainState.updateHeightEstimate(chainHeight);
chainState.statusReceived(Hash.EMPTY, UInt256.of(totalDifficulty));
when(peer.chainState()).thenReturn(chainState);
return peer;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import tech.pegasys.pantheon.ethereum.core.Block;
import tech.pegasys.pantheon.ethereum.core.BlockBody;
import tech.pegasys.pantheon.ethereum.core.BlockDataGenerator;
import tech.pegasys.pantheon.ethereum.core.BlockDataGenerator.BlockOptions;
import tech.pegasys.pantheon.ethereum.core.BlockHeader;
import tech.pegasys.pantheon.ethereum.core.TransactionReceipt;
import tech.pegasys.pantheon.ethereum.eth.manager.EthContext;
Expand Down Expand Up @@ -426,16 +427,10 @@ public void doesNotSwitchSyncTarget_betterTdUnderThreshold() {
assertThat(syncState.syncTarget().get().peer()).isEqualTo(bestPeer.getEthPeer());

// Update otherPeer so that its a better target and send some responses to push logic forward
bestPeer
.getEthPeer()
.chainState()
.updateForAnnouncedBlock(
gen.header(1000), localBlockchain.getChainHead().getTotalDifficulty().plus(201));
otherPeer
.getEthPeer()
.chainState()
.updateForAnnouncedBlock(
gen.header(1000), localBlockchain.getChainHead().getTotalDifficulty().plus(300));
final BlockHeader newBestBlock =
gen.header(1000, new BlockOptions().setDifficulty(UInt256.ZERO));
bestPeer.getEthPeer().chainState().updateForAnnouncedBlock(newBestBlock, localTd.plus(201));
otherPeer.getEthPeer().chainState().updateForAnnouncedBlock(newBestBlock, localTd.plus(300));

// Process through first task cycle
final CompletableFuture<?> firstTask = downloader.getCurrentTask();
Expand Down

0 comments on commit 18d5896

Please sign in to comment.