Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Add a maximum batch size for regular sync imports #704

Merged
merged 1 commit into from
Jun 7, 2019

Conversation

carver
Copy link
Contributor

@carver carver commented Jun 6, 2019

Without this maximum batch size, when the first batch takes a long time
to execute, the second batch will be enormous. Perhaps most importantly,
the back-pressure set by _db_buffer_capacity is never triggered if the
batch is unlimited size. So the header downloader races ahead unimpeded.

I think this was something that @veox noticed.

Note that beam sync uses the regular syncer updated in this PR, which is why it was originally bundled in #641 .

To-Do

Cute Animal Picture

put a cute animal picture link inside the parentheses

Without this maximum batch size, when the first batch takes a long time
to execute, the second batch will be enormous. Perhaps most importantly,
the back-pressure set by _db_buffer_capacity is never triggered if the
batch is unlimited size.
@carver carver mentioned this pull request Jun 7, 2019
20 tasks
@veox
Copy link
Contributor

veox commented Jun 7, 2019

I think this was something that @veox noticed.

Yes, but it seems I never filed an issue.

@carver carver requested a review from cburgdorf June 7, 2019 19:02
Copy link
Contributor

@cburgdorf cburgdorf left a comment

Choose a reason for hiding this comment

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

Oh! This is an interesting one! While working on the isolated process sync I had seen cases where the header sync just races too far ahead and I couldn't figure out how to deal with it (and the problem wasn't that apparent anymore at some point).

LGTM 👍

@carver carver merged commit 953cfb3 into ethereum:master Jun 7, 2019
@carver carver deleted the limit-block-import-batch branch June 7, 2019 21:34
carver added a commit that referenced this pull request Jun 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants