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

Revert "include small dedup in block processor to handle blockByRoot blocks" #4840

Merged
merged 1 commit into from
Apr 19, 2023

Conversation

tersec
Copy link
Contributor

@tersec tersec commented Apr 19, 2023

Reverts #4814

Syncing was broken with this: status-im/infra-nimbus#132 (comment)

@jakubgs
Copy link
Member

jakubgs commented Apr 19, 2023

As far as I can tell this fixes the bug introduced by #4814 based on investigation in:

Result after building this version:

[email protected]:~ % for port in $(seq 9304 9305); do c 0:$port/eth/v1/node/syncing | jq -c; done
{"data":{"head_slot":"6257820","sync_distance":"0","is_syncing":false,"is_optimistic":false}}
{"data":{"head_slot":"6257820","sync_distance":"0","is_syncing":false,"is_optimistic":false}}

[email protected]:~ % for port in $(seq 9304 9305); do c 0:$port/eth/v1/node/version | jq -c; done
{"data":{"version":"Nimbus/v23.3.2-75be7d-stateofus"}}
{"data":{"version":"Nimbus/v23.3.2-751d9d-stateofus"}}

Seems to be syncing fine.

@zah zah enabled auto-merge (squash) April 19, 2023 15:53
@github-actions
Copy link

Unit Test Results

         6 files   -        3       714 suites   - 357   21m 34s ⏱️ - 9m 51s
  3 360 tests  -    310    3 339 ✔️  -      52  21 💤  - 258  0 ±0 
10 438 runs   - 5 215  10 396 ✔️  - 4 952  42 💤  - 263  0 ±0 

Results for commit 6696c8f. ± Comparison against base commit 0fc8948.

@tersec tersec disabled auto-merge April 19, 2023 19:16
@tersec tersec merged commit 2246a6e into unstable Apr 19, 2023
@tersec tersec deleted the revert-4814-tbe branch April 19, 2023 19:16
@arnetheduck
Copy link
Member

hm, weird - any idea how this breaks sync?

@tersec
Copy link
Contributor Author

tersec commented Apr 22, 2023

hm, weird - any idea how this breaks sync?

#4850 -- backpressure stall. The early exit didn't do any kind of future completion, which stalls request/sync managers.

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.

3 participants