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

refactor(core-transaction-pool): better locking #3802

Merged

Conversation

rainydio
Copy link
Contributor

@rainydio rainydio commented Jun 15, 2020

Summary

Follow-up to #3800

New general-purpose Lock class is added to core-kernel utils to facilitate pool synchronization:

  • There are two kind of executions: exclusive and non-exclusive.
  • Any number of non-exclusive executions can run in parallel.
  • Only a single exclusive execution can run at a time.
  • Non-exclusive execution waits for exclusive to finish before starting.
  • Exclusive execution waits for both exclusive and non-exclusive to finish before starting.

In general this kind of locking is traditionally used in databases. Read-only transactions can run in parallel, write transactions require all read-only transactions to finish and then lock database until finished. Technique itself is more general and can be applied in non-database scenarios.

Several synchronization issues are fixed in pool:

  • Pool service now correctly ignores errors that happen in another execution while waiting for lock.
  • Sender mempool is now disposed only if it's empty and there are no pending operations waiting for lock.

Checklist

  • Tests
  • Ready to be merged

@codecov
Copy link

codecov bot commented Jun 15, 2020

Codecov Report

Merging #3802 into develop will increase coverage by 0.16%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3802      +/-   ##
===========================================
+ Coverage    96.92%   97.09%   +0.16%     
===========================================
  Files          616      617       +1     
  Lines        13999    13955      -44     
  Branches      1688     1685       -3     
===========================================
- Hits         13569    13549      -20     
+ Misses         137      117      -20     
+ Partials       293      289       -4     
Flag Coverage Δ
#functional 6.41% <0.00%> (+0.02%) ⬆️
#integration 10.13% <0.00%> (+0.03%) ⬆️
#unit 95.07% <100.00%> (+0.15%) ⬆️
Impacted Files Coverage Δ
packages/core-transaction-pool/src/utils.ts 93.75% <ø> (-2.81%) ⬇️
packages/core-kernel/src/utils/lock.ts 100.00% <100.00%> (ø)
packages/core-transaction-pool/src/mempool.ts 97.67% <100.00%> (ø)
...ckages/core-transaction-pool/src/sender-mempool.ts 100.00% <100.00%> (ø)
packages/core-transaction-pool/src/service.ts 99.18% <100.00%> (+12.62%) ⬆️
...ges/core-p2p/src/socket-server/controllers/peer.ts 98.41% <0.00%> (+0.02%) ⬆️

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 507f811...99fe584. Read the comment docs.

@rainydio rainydio changed the title refactor(core-transaction-pool): better locking in sender mempool refactor(core-transaction-pool): better locking Jun 16, 2020
@rainydio rainydio marked this pull request as ready for review June 16, 2020 04:26
@faustbrian faustbrian merged commit f0acce8 into develop Jun 17, 2020
@ghost ghost deleted the refactor/core-transaction-pool/sender-mempool-locks branch June 17, 2020 02:52
@ghost ghost removed the Status: Needs Review label Jun 17, 2020
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