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

chore(dot/sync): blockQueue refactored #2563

Merged
merged 3 commits into from
Jun 3, 2022
Merged

Conversation

qdm12
Copy link
Contributor

@qdm12 qdm12 commented May 18, 2022

Changes

  • Context aware pop
  • Do not block processReadyBlocks forever on ctx cancel
  • Change mapping to hashes set (less memory usage)
  • Remove unneeded cap int field
  • More explicit variable names in block_queue.go
  • Add unit tests

Tests

go test -tags integration github.com/ChainSafe/gossamer/dot/sync

Issues

Done whilst reviewing PR #2261

Primary Reviewer

@edwardmack

@codecov
Copy link

codecov bot commented May 18, 2022

Codecov Report

Merging #2563 (bb4138f) into development (47c23e6) will increase coverage by 0.11%.
The diff coverage is 91.30%.

@@               Coverage Diff               @@
##           development    #2563      +/-   ##
===============================================
+ Coverage        57.53%   57.64%   +0.11%     
===============================================
  Files              218      218              
  Lines            28664    28663       -1     
===============================================
+ Hits             16491    16523      +32     
+ Misses           10485    10455      -30     
+ Partials          1688     1685       -3     
Flag Coverage Δ
unit-tests 57.64% <91.30%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
dot/sync/chain_processor.go 0.00% <0.00%> (ø)
dot/sync/block_queue.go 100.00% <100.00%> (+100.00%) ⬆️
lib/runtime/wasmer/imports.go 48.40% <0.00%> (-0.05%) ⬇️
dot/network/notifications.go 65.17% <0.00%> (+1.37%) ⬆️
dot/network/inbound.go 100.00% <0.00%> (+7.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47c23e6...bb4138f. Read the comment docs.

@qdm12 qdm12 force-pushed the qdm12/dot/sync/blockqueue branch 2 times, most recently from f52f974 to 2a925c9 Compare May 18, 2022 17:30
@qdm12 qdm12 marked this pull request as ready for review May 18, 2022 20:28
Copy link
Member

@edwardmack edwardmack left a comment

Choose a reason for hiding this comment

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

This makes sense, and looks good.

@qdm12 qdm12 force-pushed the qdm12/dot/sync/blockqueue branch from 788cf07 to 2170947 Compare June 2, 2022 14:18
qdm12 added 3 commits June 2, 2022 17:44
- Context aware `pop`
- Do not block `processReadyBlocks` forever on ctx cancel
- Change mapping to hashes set (less memory usage)
- Remove unneeded `cap int` field
- More explicit variable names in block_queue.go
@qdm12 qdm12 force-pushed the qdm12/dot/sync/blockqueue branch from 2170947 to bb4138f Compare June 2, 2022 17:45
@qdm12 qdm12 merged commit ca7b252 into development Jun 3, 2022
@qdm12 qdm12 deleted the qdm12/dot/sync/blockqueue branch June 3, 2022 14:44
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