This repository was archived by the owner on Feb 23, 2022. It is now read-only.
abci: note on concurrency #258
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ebuchman
requested review from
josef-widder,
konnov and
milosevic
as code owners
konnov
approved these changes
Feb 16, 2021
Contributor
konnov
left a comment
konnov
left a comment
There was a problem hiding this comment.
This is a good warning. I only have one comment on the diff.
|
|
||
| ### Commit | ||
|
|
||
| Application state should only be persisted to disk during `Commit`. |
tac0turtle
reviewed
Feb 17, 2021
| ### Concurrency | ||
|
|
||
| In principle, each of the four ABCI connections operate concurrently with one | ||
| another. This means applications need to ensure access to state is |
Contributor
There was a problem hiding this comment.
Is this tendermint state or application. Would be good to differentiate. These two separate states get conflated often
tac0turtle
reviewed
Feb 17, 2021
tac0turtle
approved these changes
Feb 17, 2021
Contributor
tac0turtle
left a comment
tac0turtle
left a comment
There was a problem hiding this comment.
LGTM. Thank you for the addition.
The way this works seems counterintuitive. If the application handles concurrent reads they dont need to go through tendermint. It seems like applications that route reads through tendermint are not reaching their full potential.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Per the recent issue in the SDK. This note has been long overdue. Also maybe relevant to #254