Skip to content

downloader: measure successfull deliveries, not failed#17983

Merged
karalabe merged 2 commits intoethereum:masterfrom
holiman:statesync
Nov 7, 2018
Merged

downloader: measure successfull deliveries, not failed#17983
karalabe merged 2 commits intoethereum:masterfrom
holiman:statesync

Conversation

@holiman
Copy link
Copy Markdown
Contributor

@holiman holiman commented Oct 26, 2018

Sometimes, during fast-sync, there's this behaviour:

Imported new state entries  count=1556 retry=176  duplicate=527503      unexpected=1463081 
Imported new state entries  count=1536 retry=0    duplicate=527503      unexpected=1463081 
Imported new state entries  count=0    retry=208  duplicate=527544 +41  unexpected=1463248 +167

Where the number of duplicate and unexpected equals the number of retry: 41+167 = 208

Imported new state entries  count=2096 retry=208  duplicate=527544      unexpected=1463248 
Imported new state entries  count=1732 retry=0    duplicate=527544      unexpected=1463248 
Imported new state entries  count=0    retry=176  duplicate=527567 +23  unexpected=1463401 +153

153+23 = 176

Imported new state entries  count=1744 retry=0    duplicate=527567      unexpected=1463401 
Imported new state entries  count=1920 retry=0    duplicate=527567      unexpected=1463401 
Imported new state entries  count=1920 retry=0    duplicate=527567      unexpected=1463401 
Imported new state entries  count=0    retry=208  duplicate=527595 +28  unexpected=1463581 +180

180+28 = 208

The problem

The likely explanation for this is

  • We request N items from peer x (at time t0)
  • Peer x is slow in delivering them, or delivers only parts of it. We then get those items delivered from another peer.
  • In assignTasks, we now assign x with N new items, and request those from him (at time t1)
  • Now, peer x delivers the first batch (at time t2). Even though we discard everything as duplicates and unexpected, we still update his throughut measure: he delivered N items in the time t2-t1. While in actual fact, it took him t2-t0 to deliver it.

This PR changes the measurement to look at how many successfull deliveries were made, instead of the total count. A more correct approach would be to look at the correct time for the requests, but that would add a lot more tracking overhead.

Consider this a PoC, I haven't tested it extensively yet.

@holiman holiman requested a review from karalabe as a code owner October 26, 2018 09:11
Comment thread eth/downloader/statesync.go Outdated
Comment thread eth/downloader/statesync.go Outdated
Copy link
Copy Markdown
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

LGTM

@karalabe karalabe added this to the 1.8.18 milestone Nov 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants