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.

Sign up for GitHub

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

Jump to bottom

Send reduction concurrently without blocking votes re-propagation #1442

Merged
merged 23 commits into from
Jul 28, 2022
Merged

Send reduction concurrently without blocking votes re-propagation #1442

merged 23 commits into from
Jul 28, 2022

Conversation

goshawk-3
Copy link
Contributor

@goshawk-3 goshawk-3 commented Jul 14, 2022 •
edited
Loading

fixes #1441

Additionally, another edge case will be addressed out of this PR (with issue #1444)

@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2022 •
edited
Loading

Codecov Report

Merging #1442 (c216119) into master (828364b) will decrease coverage by 10.29%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #1442       +/-   ##
===========================================
- Coverage   50.51%   40.22%   -10.30%     
===========================================
  Files         145      146        +1     
  Lines        7544     9646     +2102     
===========================================
+ Hits         3811     3880       +69     
- Misses       3066     5138     +2072     
+ Partials      667      628       -39
Impacted Files Coverage Δ
pkg/core/consensus/testing/node.go 0.00% <0.00%> (-93.55%) ⬇️
...ore/consensus/blockgenerator/candidate/testutil.go 0.00% <0.00%> (-90.91%) ⬇️
pkg/core/consensus/testing/utils.go 0.00% <0.00%> (-75.61%) ⬇️
...core/consensus/blockgenerator/candidate/genesis.go 0.00% <0.00%> (-69.24%) ⬇️
...nsensus/blockgenerator/candidate/blockgenerator.go 0.00% <0.00%> (-64.29%) ⬇️
pkg/gql/query/transactions.go 13.43% <0.00%> (-52.55%) ⬇️
pkg/rpc/client/session.go 21.42% <0.00%> (-47.15%) ⬇️
pkg/core/consensus/reduction/aggregator.go 47.36% <0.00%> (-43.94%) ⬇️
pkg/util/nativeutils/eventbus/eventbus.go 57.14% <0.00%> (-42.86%) ⬇️
pkg/rpc/client/client.go 3.73% <0.00%> (-36.61%) ⬇️
... and 168 more

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 3ebd6cb...c216119. Read the comment docs.

@goshawk-3 goshawk-3 marked this pull request as ready for review July 15, 2022 07:55
@goshawk-3 goshawk-3 marked this pull request as draft July 20, 2022 08:50
@goshawk-3
Pass context to VerifyCandidateBlock method
1c01f5e
@goshawk-3
- Implement SendReductionAsync
02de1ad 
- Protect reduction.verifiedHash with a mutex
- Remove r.Republish from SendReduction
@goshawk-3
Use waitgroup to wait for reduction sending completion
c1c0a4c
@goshawk-3
Use waitgroup to wait for reduction sending completion
0b866af
@goshawk-3
Fix unit test
03cea1c
@goshawk-3
Implement SafeSet wrapper
4024fd5
@goshawk-3
Cache VST calls results
e9a0eff
@goshawk-3
Remove wait and cancel functions for firststep reduction
4c48f9e
@goshawk-3
Remove wait function for secondstep reduction
8622e35
@goshawk-3
Remove ThrotteMilli on block acceptance. Propagate block before accep…
e019981 
…ting
@goshawk-3
[experimental] Pre-execute VST call
c284b4a
@goshawk-3
Republish reduction on VST completion
84ea932
@goshawk-3
Rollback extraDelay sleep in EST calls
97d7a1f
@goshawk-3
Remove the obsolete verifiedHash function
d941222
@goshawk-3
Implement AsyncSend wrapper
f6e2202
@goshawk-3
Use asyncsend.Go to send reduction concurrently
ee11bbe
@goshawk-3 goshawk-3 marked this pull request as ready for review July 27, 2022 13:23
@goshawk-3 goshawk-3 mentioned this pull request Jul 27, 2022
Closed
@herr-seppia herr-seppia self-assigned this Jul 27, 2022
herr-seppia
herr-seppia reviewed Jul 27, 2022
View reviewed changes
Copy link
Member

@herr-seppia herr-seppia left a comment

Choose a reason for hiding this comment

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

Not sure about some code snippets

pkg/core/consensus/reduction/firststep/step.go Outdated Show resolved Hide resolved
pkg/core/consensus/reduction/secondstep/step.go Outdated Show resolved Hide resolved
pkg/core/chain/chain.go Show resolved Hide resolved
pkg/core/consensus/reduction/firststep/step.go Outdated Show resolved Hide resolved
@herr-seppia herr-seppia removed their assignment Jul 27, 2022
herr-seppia
herr-seppia approved these changes Jul 28, 2022
View reviewed changes
Copy link
Member

@herr-seppia herr-seppia left a comment

Choose a reason for hiding this comment

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

LGTM

@goshawk-3 goshawk-3 merged commit 398f63f into master Jul 28, 2022
@goshawk-3 goshawk-3 deleted the concurrent_voting branch July 28, 2022 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@herr-seppia herr-seppia herr-seppia approved these changes

@autholykos autholykos Awaiting requested review from autholykos

@t00ts t00ts Awaiting requested review from t00ts

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Investigate consensus delay when mempool is full
3 participants
@goshawk-3 @codecov-commenter @herr-seppia