Skip to content

enqueue missing parent block if stored in local DB#6122

Merged
etan-status merged 3 commits into
unstablefrom
dev/etan/df-knownmissing
Mar 22, 2024
Merged

enqueue missing parent block if stored in local DB#6122
etan-status merged 3 commits into
unstablefrom
dev/etan/df-knownmissing

Conversation

@etan-status

Copy link
Copy Markdown
Contributor

When checking for MissingParent, it may be that the parent block was already discovered as part of a prior run. In that case, it can be loaded from storage and processed without having to rediscover the entire branch from the network. This is similar to #6112 but for blocks that are discovered via gossip / sync mgr instead of via request mgr.

When checking for `MissingParent`, it may be that the parent block was
already discovered as part of a prior run. In that case, it can be
loaded from storage and processed without having to rediscover the
entire branch from the network. This is similar to #6112 but for blocks
that are discovered via gossip / sync mgr instead of via request mgr.
@github-actions

github-actions Bot commented Mar 22, 2024

Copy link
Copy Markdown

Unit Test Results

         9 files  ±0    1 115 suites  ±0   31m 15s ⏱️ - 2m 48s
  4 244 tests ±0    3 897 ✔️ ±0  347 💤 ±0  0 ±0 
16 926 runs  ±0  16 528 ✔️ ±0  398 💤 ±0  0 ±0 

Results for commit 94b95c1. ± Comparison against base commit 17ee40b.

♻️ This comment has been updated with latest results.

if parent.error() == VerifierError.MissingParent:
let
parent_root = signedBlock.message.parent_root
parentBlck = dag.getForkedBlock(parent_root)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why would we have the block here if checkHeadBlock fails with missing parent?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ie that would indicate that a race happened between index and block write which should be .. rare?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It happens when there are non-canonical branches from a previous run of the beacon node. We only assign BlockRef to the canonical chain on startup while rediscovering all other branches via sync / request manager. If non-canonical branches have significant length, that takes a very long time.

checkParentRoot actually already logs this case differently from the regular case:

  let parent = dag.getBlockRef(blck.parent_root).valueOr:
    # There are two cases where the parent won't be found: we don't have it or
    # it has been finalized already, and as a result the branch the new block
    # is on is no longer a viable fork candidate - we can't tell which is which
    # at this stage, but we can check if we've seen the parent block previously
    # and thus prevent requests for it to be downloaded again.
    let parentId = dag.getBlockId(blck.parent_root)
    if parentId.isSome() and parentId.get.slot < dag.finalizedHead.slot:
      debug "Block unviable due to pre-finalized-checkpoint parent",
        parentId = parentId.get()
      return err(VerifierError.UnviableFork)

    debug "Block parent unknown or finalized already", parentId
    return err(VerifierError.MissingParent)

parentId is set to some in case that the block is already available in the database. So, let's just load it from there to speed up discovery of other branches.

@etan-status etan-status merged commit 2d9586a into unstable Mar 22, 2024
@etan-status etan-status deleted the dev/etan/df-knownmissing branch March 22, 2024 13:35
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