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

go/worker/executor: don't hold the lock during storage requests #3320

Merged
merged 1 commit into from
Sep 24, 2020

Conversation

ptrus
Copy link
Member

@ptrus ptrus commented Sep 24, 2020

Don't unnecessarily hold the lock during external requests to storage nodes, as those can take long time.

Issue discovered in the daily tests with long restarts, when the storage node is taken offline, these requests can take up to 15 sec to timeout.

@ptrus ptrus force-pushed the ptrus/fix/executor-dispatch-lock branch 2 times, most recently from 9599c46 to 9015b00 Compare September 24, 2020 11:55
@ptrus ptrus force-pushed the ptrus/fix/executor-dispatch-lock branch from 9015b00 to 13b2407 Compare September 24, 2020 12:06
@ptrus ptrus force-pushed the ptrus/fix/executor-dispatch-lock branch from 13b2407 to 7b573f5 Compare September 24, 2020 12:32
@codecov
Copy link

codecov bot commented Sep 24, 2020

Codecov Report

Merging #3320 into master will increase coverage by 0.51%.
The diff coverage is 55.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3320      +/-   ##
==========================================
+ Coverage   64.89%   65.40%   +0.51%     
==========================================
  Files         371      371              
  Lines       33045    33062      +17     
==========================================
+ Hits        21443    21623     +180     
+ Misses       8404     8267     -137     
+ Partials     3198     3172      -26     
Impacted Files Coverage Δ
go/worker/compute/executor/committee/node.go 64.10% <55.00%> (+3.25%) ⬆️
go/common/grpc/errors.go 79.06% <0.00%> (-9.31%) ⬇️
go/worker/common/p2p/dispatch.go 74.52% <0.00%> (-4.72%) ⬇️
...n/crypto/signature/signers/memory/memory_signer.go 72.41% <0.00%> (-3.45%) ⬇️
go/keymanager/client/client.go 81.41% <0.00%> (-1.77%) ⬇️
go/worker/common/committee/node.go 74.86% <0.00%> (ø)
go/consensus/tendermint/roothash/roothash.go 70.76% <0.00%> (ø)
go/worker/storage/committee/checkpoint_sync.go 73.38% <0.00%> (ø)
go/runtime/host/protocol/connection.go 57.20% <0.00%> (+0.82%) ⬆️
go/consensus/tendermint/full/full.go 60.58% <0.00%> (+0.92%) ⬆️
... and 26 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 c2355a8...7b573f5. Read the comment docs.

@ptrus ptrus merged commit f8f3ce0 into master Sep 24, 2020
@ptrus ptrus deleted the ptrus/fix/executor-dispatch-lock branch September 24, 2020 12:57
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.

None yet

2 participants