Skip to content

when adding duplicates to quarantine, schedule deepest missing parent#6112

Merged
etan-status merged 2 commits into
unstablefrom
dev/etan/rq-parent
Mar 21, 2024
Merged

when adding duplicates to quarantine, schedule deepest missing parent#6112
etan-status merged 2 commits into
unstablefrom
dev/etan/rq-parent

Conversation

@etan-status

Copy link
Copy Markdown
Contributor

During sync, sometimes the same block gets encountered and added to quarantine multiple times. If its parent is already known, quarantine incorrectly registers it as missing, leading to re-download. This can be fixed by registering the parent's deepest missing parent recursively.

Also increase the stickiness of missing. We only perform 4 attempts within ~16 seconds before giving up. Very frequently, this is not enough and there is no progress until sync manager kicks in even on holesky.

During sync, sometimes the same block gets encountered and added to
quarantine multiple times. If its parent is already known, quarantine
incorrectly registers it as missing, leading to re-download. This can
be fixed by registering the parent's deepest missing parent recursively.

Also increase the stickiness of `missing`. We only perform 4 attempts
within ~16 seconds before giving up. Very frequently, this is not enough
and there is no progress until sync manager kicks in even on holesky.
@github-actions

Copy link
Copy Markdown

Unit Test Results

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

Results for commit 18e56bc.

@etan-status etan-status enabled auto-merge (squash) March 21, 2024 16:18
@etan-status etan-status merged commit 12a2f8c into unstable Mar 21, 2024
@etan-status etan-status deleted the dev/etan/rq-parent branch March 21, 2024 18:41
etan-status added a commit that referenced this pull request Mar 22, 2024
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.
etan-status added a commit that referenced this pull request Mar 22, 2024
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.
@arnetheduck

Copy link
Copy Markdown
Member

This can be fixed by registering the parent's deepest missing parent recursively.

why is this needed though? that ancestor should already be registered in this case?

@etan-status

Copy link
Copy Markdown
Contributor Author

Entries from the missing list expire if not found in a handful of attempts.

@arnetheduck

Copy link
Copy Markdown
Member

Entries from the missing list expire if not found in a handful of attempts.

this sounds like a broader issue then - ie in the quarantine, we have limited space (in part because it lives in-memory) - it seems to me then that when running low on space, we should strive to keep "early" entries and discard entries that build on top of them first so that we don't inadvertedly remove an ancestor over a descendant from the list of "interesting" blocks -> the "retry" count of an ancestor should be the sum of all its known descendants and used to prioritise requests (since resolving such a quarantine item would have the effect of unblocking a large number of blocks)

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