Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make preload continue with peers when block download failed #636

Merged

Conversation

@moreal
Copy link
Contributor

moreal commented Oct 30, 2019

It was aborted if block-download peer became unavailable though it can download blocks from another peers. so it became to continue block-download with another peers if changed.

This pr might be related with #634.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 30, 2019

Codecov Report

Merging #636 into master will increase coverage by 0.12%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #636      +/-   ##
==========================================
+ Coverage   88.13%   88.25%   +0.12%     
==========================================
  Files         217      217              
  Lines       17181    17248      +67     
==========================================
+ Hits        15143    15223      +80     
+ Misses       1162     1145      -17     
- Partials      876      880       +4
Impacted Files Coverage Δ
Libplanet/Net/Swarm.cs 85.9% <100%> (+0.95%) ⬆️
Libplanet.Tests/Net/SwarmTest.cs 98.79% <100%> (+0.02%) ⬆️
Libplanet/Net/BlockDownloadState.cs 100% <100%> (ø) ⬆️
Libplanet/Store/LiteDBStore.cs 89.3% <0%> (+0.44%) ⬆️
@moreal moreal force-pushed the moreal:retry-with-another-peer-when-preload-failed branch 6 times, most recently from f034079 to 4937a5e Oct 30, 2019
@moreal moreal marked this pull request as ready for review Nov 1, 2019
@moreal moreal self-assigned this Nov 1, 2019
Libplanet/Net/Swarm.cs Outdated Show resolved Hide resolved
Libplanet.Tests/Net/SwarmTest.cs Outdated Show resolved Hide resolved
Libplanet/Net/Swarm.cs Outdated Show resolved Hide resolved
Libplanet/Net/Swarm.cs Outdated Show resolved Hide resolved
Copy link
Member

longfin left a comment

Build failed.


"/home/vsts/work/1/s/Libplanet.sln" (default target) (1:2) ->
"/home/vsts/work/1/s/Libplanet/Libplanet.csproj" (default target) (4:16) ->
"/home/vsts/work/1/s/Libplanet/Libplanet.csproj" (Build target) (4:17) ->
(CoreCompile target) -> 
  Net/Swarm.cs(1786,33): error SA1508: A closing brace should not be preceded by a blank line. [/home/vsts/work/1/s/Libplanet/Libplanet.csproj]


"/home/vsts/work/1/s/Libplanet.sln" (default target) (1:2) ->
"/home/vsts/work/1/s/Libplanet/Libplanet.csproj" (default target) (4:16) ->
"/home/vsts/work/1/s/Libplanet/Libplanet.csproj" (Build target) (4:18) ->
  Net/Swarm.cs(1786,33): error SA1508: A closing brace should not be preceded by a blank line. [/home/vsts/work/1/s/Libplanet/Libplanet.csproj]

    0 Warning(s)
    2 Error(s)
@moreal moreal force-pushed the moreal:retry-with-another-peer-when-preload-failed branch 4 times, most recently from f848cd0 to a156fb8 Nov 1, 2019
@moreal moreal requested review from dahlia, longfin, limebell and earlbread Nov 1, 2019
@longfin

This comment has been minimized.

Copy link
Member

longfin commented Nov 1, 2019

Test failed. (see also)

 Libplanet.Tests.Net.SwarmTest.PreloadContinueWithNextPeers [FAIL]
      Assert.Equal() Failure
      Expected: BlockChain<DumbAction> [1ee57f005a9fd51233cbec5a243e00eaae468789bbbcdced365522b438ee3003, 1a6655a401d22759ce24c2528c27f538ccbdc67128de40a49ecf6e1cf91b0500, 99ace77bc6749bcfeba99f92de9106f3e263c3d963062d88ed2e223c2c3e0b00, 93f94c04d1ca34a8e066af1c66e752ea5d478a27b69f714467949779525c0700, 58e73162452ab7d0903f46f9232ac97ae6340ca5f98e854c5461eefa84371500, ...]
      Actual:   BlockChain<DumbAction> []
      Stack Trace:
          at Libplanet.Tests.Net.SwarmTest.PreloadContinueWithNextPeers () [0x00394] in <d08b74ef784f46f086209436045ac815>:0 
@moreal moreal force-pushed the moreal:retry-with-another-peer-when-preload-failed branch 2 times, most recently from 2ed8665 to 11ee1b9 Nov 4, 2019
@moreal moreal force-pushed the moreal:retry-with-another-peer-when-preload-failed branch from 11ee1b9 to 3e89ae9 Nov 4, 2019
@moreal

This comment has been minimized.

Copy link
Contributor Author

moreal commented Nov 4, 2019

Test failed. (see also)

 Libplanet.Tests.Net.SwarmTest.PreloadContinueWithNextPeers [FAIL]
      Assert.Equal() Failure
      Expected: BlockChain<DumbAction> [1ee57f005a9fd51233cbec5a243e00eaae468789bbbcdced365522b438ee3003, 1a6655a401d22759ce24c2528c27f538ccbdc67128de40a49ecf6e1cf91b0500, 99ace77bc6749bcfeba99f92de9106f3e263c3d963062d88ed2e223c2c3e0b00, 93f94c04d1ca34a8e066af1c66e752ea5d478a27b69f714467949779525c0700, 58e73162452ab7d0903f46f9232ac97ae6340ca5f98e854c5461eefa84371500, ...]
      Actual:   BlockChain<DumbAction> []
      Stack Trace:
          at Libplanet.Tests.Net.SwarmTest.PreloadContinueWithNextPeers () [0x00394] in <d08b74ef784f46f086209436045ac815>:0 

@longfin It's fixed. It was because Swarm<T>.FillBlockAsync became not to throw exception in #644.

Can you review this again?

/// <summary>
/// The peer which sent the block.
/// </summary>
public BoundPeer SourcePeer { get; internal set; }

This comment has been minimized.

Copy link
@earlbread

earlbread Nov 4, 2019

Member

What is this used for?

This comment has been minimized.

Copy link
@moreal

moreal Nov 4, 2019

Author Contributor

I added it to debug easier because sender(peer) can be changed during preload by this pull-request.

CHANGES.md Outdated Show resolved Hide resolved
Libplanet/Net/Swarm.cs Outdated Show resolved Hide resolved
Libplanet/Net/Swarm.cs Outdated Show resolved Hide resolved
Libplanet/Net/Swarm.cs Outdated Show resolved Hide resolved
@moreal moreal dismissed stale reviews from earlbread and longfin via ac987c6 Nov 6, 2019
@dahlia
dahlia approved these changes Nov 6, 2019
@moreal moreal requested review from longfin and earlbread Nov 6, 2019
CHANGES.md Outdated
@@ -80,6 +81,8 @@ To be released.
incomplete states from the beginning if necessary. [[#645]]
- `IStore.PutBlock<T>()` became to do nothing when it takes
the `Block<T>` more than once. [[#647]]
- `PreloadAsync()` became to try downloading blocks from all neighbor peers,

This comment has been minimized.

Copy link
@dahlia

dahlia Nov 6, 2019

Member

Oh, you missed the class name.

Suggested change
- `PreloadAsync()` became to try downloading blocks from all neighbor peers,
- `Swarm<T>.PreloadAsync()` became to try downloading blocks from all neighbor peers,

This comment has been minimized.

Copy link
@moreal

moreal Nov 6, 2019

Author Contributor

Oops 😅, I fixed!

@moreal moreal force-pushed the moreal:retry-with-another-peer-when-preload-failed branch from ac987c6 to 186f3ae Nov 6, 2019
@longfin
longfin approved these changes Nov 6, 2019
@dahlia
dahlia approved these changes Nov 6, 2019
@moreal moreal merged commit 1357188 into planetarium:master Nov 6, 2019
18 checks passed
18 checks passed
dist
Details
docs
Details
WIP Ready for review
Details
changelog This contains self-describing changelog.
Details
codecov/patch 100% of diff hit (target 88.13%)
Details
codecov/project 88.25% (+0.12%) compared to 0040d74
Details
license/cla Contributor License Agreement is signed.
Details
planetarium.libplanet Build #20191106.5 succeeded
Details
planetarium.libplanet (Linux_Mono) Linux_Mono succeeded
Details
planetarium.libplanet (Linux_NETCore) Linux_NETCore succeeded
Details
planetarium.libplanet (Windows_Mono) Windows_Mono succeeded
Details
planetarium.libplanet (Windows_NETCore) Windows_NETCore succeeded
Details
planetarium.libplanet (Windows_NETCore_Benchmark) Windows_NETCore_Benchmark succeeded
Details
planetarium.libplanet (Windows_NETCore_coverage) Windows_NETCore_coverage succeeded
Details
planetarium.libplanet (Windows_NETFramework) Windows_NETFramework succeeded
Details
planetarium.libplanet (macOS_Mono) macOS_Mono succeeded
Details
planetarium.libplanet (macOS_NETCore) macOS_NETCore succeeded
Details
planetarium.libplanet (macOS_Unity) macOS_Unity succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.