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

Initial cut height limit and private flag #674

Merged
merged 9 commits into from
Nov 11, 2019

Conversation

larskuhtz
Copy link
Contributor

@larskuhtz larskuhtz commented Nov 8, 2019

  • private: true: prevents the addition of any new peers to the PeerDb. The only peers that a private node connects to are the configured known peers. With this PR other nodes can still push cuts to a private node. PR require cut origin to be known #681 prevents unknown peers from pushing cuts.

  • initialCutHeightLimit: When switching between forks, long rewinds are very expensive. Another issue is that a node that switches from fork A to B will go to B at a height larger than the current height an A. If the fork point is deep down it will attempt to do this in a single step, which might cause timeouts during rewind or pact replay (which doesn't cache intermediate states).

    This PR allows the user to provide an estimate of the fork point or any lower bound (0 is always a safe, but inefficient choice), to cause he node to start cut processing at a block height blow the fork point.

    Another benefit of this option is, that it allows a node to re-validate the history of the block chain from some point onwards. This verifies the consistency of the database and the chain.

@fosskers fosskers added the component: cuts The Cut processing pipeline. label Nov 8, 2019
@fosskers
Copy link
Contributor

fosskers commented Nov 9, 2019

This should go in after #677 .

@@ -376,6 +380,9 @@ pChainwebConfiguration = id
<*< configCutFetchTimeout .:: option auto
% long "cut-fetch-timeout"
<> help "After this many microseconds, give up trying to sync a particular Cut"
<*< configInitialCutHeightLimit .:: fmap (Just . BlockHeight) % option auto
Copy link
Member

Choose a reason for hiding this comment

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

should this even be configurable? This sounds like best practice for catchup. We should poll our bootstraps for their most recent highest cut to determine this number on startup and defer to 0 otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this to be effective, we need an estimate of the height of the fork point. The better the estimate, the better the performance. 0 is always a valid estimate, but it's inefficient, because pact replay takes a long time -- in particular once we have transactions in the system.

To me it seems that those extreme scenarios with very deep forks are best resolved by manually switching to an "repair" mode. This options plus the --private flag would be such a "repair" mode.

@larskuhtz larskuhtz changed the title add cut initial height limit to cut db config Initial cut height limit and private flag Nov 10, 2019
@larskuhtz larskuhtz requested a review from emilypi November 10, 2019 23:24
@larskuhtz larskuhtz requested a review from fosskers November 11, 2019 03:22
@larskuhtz larskuhtz merged commit d9817e0 into master Nov 11, 2019
@larskuhtz larskuhtz deleted the lars/cutdb-initial-height-limit branch November 11, 2019 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: cuts The Cut processing pipeline.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants