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/compute: Improve proposed transaction handling #4640

Merged
merged 3 commits into from Apr 5, 2022

Conversation

kostko
Copy link
Member

@kostko kostko commented Apr 4, 2022

No description provided.

@kostko kostko added the c:bug Category: bug label Apr 4, 2022
@kostko kostko force-pushed the kostko/fix/txpool-sync branch 9 times, most recently from 510c9ad to 3669db3 Compare April 5, 2022 07:28
@kostko kostko marked this pull request as ready for review April 5, 2022 07:36
@codecov
Copy link

codecov bot commented Apr 5, 2022

Codecov Report

Merging #4640 (6670997) into master (2caa3ac) will increase coverage by 0.05%.
The diff coverage is 92.50%.

❗ Current head 6670997 differs from pull request most recent head bfd33c3. Consider uploading reports for the commit bfd33c3 to get more accurate results

@@            Coverage Diff             @@
##           master    #4640      +/-   ##
==========================================
+ Coverage   67.05%   67.10%   +0.05%     
==========================================
  Files         431      431              
  Lines       48818    48870      +52     
==========================================
+ Hits        32735    32796      +61     
+ Misses      12050    12049       -1     
+ Partials     4033     4025       -8     
Impacted Files Coverage Δ
go/worker/compute/executor/committee/batch.go 75.75% <ø> (+3.75%) ⬆️
go/worker/common/p2p/txsync/client.go 68.75% <81.25%> (+10.41%) ⬆️
go/worker/compute/executor/committee/node.go 72.00% <87.50%> (-0.38%) ⬇️
go/worker/common/p2p/rpc/client.go 80.20% <88.57%> (+0.42%) ⬆️
go/runtime/txpool/txpool.go 76.20% <97.72%> (+2.20%) ⬆️
.../worker/compute/executor/committee/transactions.go 85.48% <100.00%> (+10.09%) ⬆️
go/oasis-node/cmd/common/metrics/disk.go 65.51% <0.00%> (-20.69%) ⬇️
go/oasis-node/cmd/common/metrics/resource.go 84.00% <0.00%> (-8.00%) ⬇️
go/consensus/api/submission.go 64.28% <0.00%> (-4.77%) ⬇️
go/storage/mkvs/insert.go 87.75% <0.00%> (-2.73%) ⬇️
... and 27 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 57c55f5...bfd33c3. Read the comment docs.

@@ -303,9 +359,10 @@ func (t *txPool) GetPrioritizedBatch(offset *hash.Hash, limit uint32) []*transac

func (t *txPool) GetKnownBatch(batch []hash.Hash) ([]*transaction.CheckedTransaction, map[hash.Hash]int) {
t.schedulerLock.Lock()
defer t.schedulerLock.Unlock()
schedulerQueue := t.schedulerQueue
Copy link
Contributor

Choose a reason for hiding this comment

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

This is spooky, I take it this is to avoid adding GetKnownBatchLocked?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

go/runtime/txpool/txpool.go Show resolved Hide resolved
@kostko kostko merged commit 126ce23 into master Apr 5, 2022
@kostko kostko deleted the kostko/fix/txpool-sync branch April 5, 2022 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:bug Category: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants