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

feat: concurrent checkTx #141

Merged
merged 5 commits into from
Apr 22, 2021
Merged

Conversation

jinsan-line
Copy link
Contributor

@jinsan-line jinsan-line commented Apr 22, 2021

Epic: line/lbm#1263

Cherry-pick: #49, #51

Description

To optimize performance, we need to increase concurrency. As a first step for it, I implement concurrent checkTx. The key change is to remove app.mtx.Lock() from the abci local client. An application, as an abci server, is better to protect itself from concurrency than an abci client. W/ current implementation, the abci local client protects an abci server but it decreases concurrency.

CONTRACT:

Now, an application should protect itself from concurrent checkTx as an abci server that means it should be thread safe.
We'll also increase concurrency for other abci methods as much as possible in the future.

What should application protect?

In lbm-sdk application, it should protect accounts from the concurrency.

Motivation and context

How has this been tested?

  • make test
  • make test-cover

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

* feat: implement new abci, `BeginRecheckTx()` and `EndRecheckTx()`

* test: fix tests

* refactor: decompose checkTx & runTx

* chore: protect app.checkState w/ RWMutex for simulate

* chore: remove unused var

* feat: account lock decorator

* chore: skip AccountLockDecorator if not checkTx

* chore: bump up tendermint

* chore: revise accountlock position

* chore: accountlock_test

* chore: revise accountlock covers `cache.Write()`

* chore: revise `sampleBytes` to `2`

* fix: test according to `sampleBytes`

* chore: revise `getUniqSortedAddressKey()` and add `getAddressKey()`

* chore: revise `how to sort` not to use `reflection`

* chore: bump up tendermint

* test: check `sorted` in `TestGetUniqSortedAddressKey()`

* chore: move `accountLock` from `anteTx()` to `checkTx()`
# Conflicts:
#	baseapp/abci.go
#	baseapp/baseapp.go
#	baseapp/baseapp_test.go
#	baseapp/helpers.go
#	go.mod
#	go.sum
#	x/bank/bench_test.go
#	x/mock/test_utils.go
* fix: gasWanted & gasUsed is always `0`

* chore: error log for general panic
# Conflicts:
#	baseapp/baseapp.go
@jinsan-line jinsan-line added this to the Initail ebony milestone Apr 22, 2021
@jinsan-line jinsan-line self-assigned this Apr 22, 2021
@jinsan-line jinsan-line merged commit a25ca2f into v2/develop Apr 22, 2021
@jinsan-line jinsan-line deleted the jinsan/v2/concurrent-check-tx branch April 22, 2021 07:41
brew0722 pushed a commit that referenced this pull request Apr 26, 2021
@dudong2 dudong2 mentioned this pull request Nov 8, 2022
4 tasks
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