fix!: remove grpc query routing through tendermint #10045
Merged
Conversation
This file contains 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
This reverts commit e306e5b.
tac0turtle
changed the title
tac0turtle
changed the title
ValarDragon
reviewed
Sep 1, 2021
ValarDragon
reviewed
Sep 1, 2021
tac0turtle
changed the title
4 tasks
|
Visit https://dashboard.github.orijtech.com?back=0&pr=10045&remote=false&repo=cosmos%2Fcosmos-sdk to see benchmark details. |
Codecov Report
@@ Coverage Diff @@
## master #10045 +/- ##
=======================================
Coverage 63.66% 63.66%
=======================================
Files 573 573
Lines 53816 53795 -21
=======================================
- Hits 34260 34251 -9
+ Misses 17607 17596 -11
+ Partials 1949 1948 -1
|
This was referenced Sep 3, 2021
4 tasks
|
+1 for this PR! This fix may prove useful to relayer operators. Some relayers (Hermes in particular) tend to put a lot of pressure on the RPC+GRPC endpoints of full nodes, making it necessary to use relatively beefy machines. (As a side note, there's also some anecdotal evidence that the gRPC endpoint is not very reliable, e.g., informalsystems/hermes#1220 but unclear if related to routing via tendermint.) Any estimate on when this will land into SDK line(s)? |
alexanderbez
approved these changes
Sep 9, 2021
There was a problem hiding this comment.
@marbar3778 can you please address @ValarDragon's comments before merging?
19 tasks
|
Until know, the gRPC server just called into Tendermint RPC, which then called the ABCI app to make the query. In CosmJS, we figured this was extra overhead and just called Tendermint RPC directly with those queries, and this worked great. (Especially as the gRPC server was quite buggy until March 2021 or so). This PR now means there are two different paths - gRPC -> ABCI and Tendermint RPC -> ABCI, which different characteristics? Can you explain what the difference between these two is after this PR? (eg. will we have trouble to keep calling Tendermint RPC directly?) |
This PR reverts routing grpc queries through tendermint. Now it should go directly to the store instead of tendermint. Ideally, apps are queryable without tendermint as this can block other parts of tendermint from starting |
mergify bot
pushed a commit
that referenced
this pull request
Sep 30, 2021
<!-- The default pull request template is for types feat, fix, or refactor. For other templates, add one of the following parameters to the url: - template=docs.md - template=other.md --> ## Description Revert routing queries through tendermint. The reason this change was made is because of this error: ``` fatal error: concurrent map read and map write ``` The person who identified this error submitted these steps to reproduce ``` User sends a query with grpc Tendermint Commit new blocks and tries to Commit IAVL (which causing IAVL versions map to change) At the same time query tries to read from the same map (iavl.(*MutableTree).VersionExists) to check if requested version is exists Node exits with fatal error: concurrent map read and map write ``` With the recent changes to IAVL submitted by terra (cc @YunSuk-Yeo) the reason for why we need to route through tendermint is no longer present. We should revert it when 0.17.1 of IAVL is cut, which will be later today. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 8091846) # Conflicts: # CHANGELOG.md
amaury1093
pushed a commit
that referenced
this pull request
Oct 4, 2021
…10269) * fix!: remove grpc query routing through tendermint (#10045) <!-- The default pull request template is for types feat, fix, or refactor. For other templates, add one of the following parameters to the url: - template=docs.md - template=other.md --> ## Description Revert routing queries through tendermint. The reason this change was made is because of this error: ``` fatal error: concurrent map read and map write ``` The person who identified this error submitted these steps to reproduce ``` User sends a query with grpc Tendermint Commit new blocks and tries to Commit IAVL (which causing IAVL versions map to change) At the same time query tries to read from the same map (iavl.(*MutableTree).VersionExists) to check if requested version is exists Node exits with fatal error: concurrent map read and map write ``` With the recent changes to IAVL submitted by terra (cc @YunSuk-Yeo) the reason for why we need to route through tendermint is no longer present. We should revert it when 0.17.1 of IAVL is cut, which will be later today. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 8091846) # Conflicts: # CHANGELOG.md * fix conflicts * revert Co-authored-by: Marko <marbar3778@yahoo.com>
4 tasks
evan-forbes
pushed a commit
to evan-forbes/cosmos-sdk
that referenced
this pull request
Oct 12, 2021
…0045) (cosmos#10269) * fix!: remove grpc query routing through tendermint (cosmos#10045) <!-- The default pull request template is for types feat, fix, or refactor. For other templates, add one of the following parameters to the url: - template=docs.md - template=other.md --> ## Description Revert routing queries through tendermint. The reason this change was made is because of this error: ``` fatal error: concurrent map read and map write ``` The person who identified this error submitted these steps to reproduce ``` User sends a query with grpc Tendermint Commit new blocks and tries to Commit IAVL (which causing IAVL versions map to change) At the same time query tries to read from the same map (iavl.(*MutableTree).VersionExists) to check if requested version is exists Node exits with fatal error: concurrent map read and map write ``` With the recent changes to IAVL submitted by terra (cc @YunSuk-Yeo) the reason for why we need to route through tendermint is no longer present. We should revert it when 0.17.1 of IAVL is cut, which will be later today. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 8091846) # Conflicts: # CHANGELOG.md * fix conflicts * revert Co-authored-by: Marko <marbar3778@yahoo.com>
evan-forbes
pushed a commit
to evan-forbes/cosmos-sdk
that referenced
this pull request
Nov 1, 2021
…0045) (cosmos#10269) * fix!: remove grpc query routing through tendermint (cosmos#10045) <!-- The default pull request template is for types feat, fix, or refactor. For other templates, add one of the following parameters to the url: - template=docs.md - template=other.md --> ## Description Revert routing queries through tendermint. The reason this change was made is because of this error: ``` fatal error: concurrent map read and map write ``` The person who identified this error submitted these steps to reproduce ``` User sends a query with grpc Tendermint Commit new blocks and tries to Commit IAVL (which causing IAVL versions map to change) At the same time query tries to read from the same map (iavl.(*MutableTree).VersionExists) to check if requested version is exists Node exits with fatal error: concurrent map read and map write ``` With the recent changes to IAVL submitted by terra (cc @YunSuk-Yeo) the reason for why we need to route through tendermint is no longer present. We should revert it when 0.17.1 of IAVL is cut, which will be later today. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 8091846) # Conflicts: # CHANGELOG.md * fix conflicts * revert Co-authored-by: Marko <marbar3778@yahoo.com>
19 tasks
4 tasks
4 tasks
zakir-code
pushed a commit
to FunctionX/cosmos-sdk
that referenced
this pull request
Apr 14, 2022
…0045) (cosmos#10269) * fix!: remove grpc query routing through tendermint (cosmos#10045) <!-- The default pull request template is for types feat, fix, or refactor. For other templates, add one of the following parameters to the url: - template=docs.md - template=other.md --> ## Description Revert routing queries through tendermint. The reason this change was made is because of this error: ``` fatal error: concurrent map read and map write ``` The person who identified this error submitted these steps to reproduce ``` User sends a query with grpc Tendermint Commit new blocks and tries to Commit IAVL (which causing IAVL versions map to change) At the same time query tries to read from the same map (iavl.(*MutableTree).VersionExists) to check if requested version is exists Node exits with fatal error: concurrent map read and map write ``` With the recent changes to IAVL submitted by terra (cc @YunSuk-Yeo) the reason for why we need to route through tendermint is no longer present. We should revert it when 0.17.1 of IAVL is cut, which will be later today. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 8091846) # Conflicts: # CHANGELOG.md * fix conflicts * revert Co-authored-by: Marko <marbar3778@yahoo.com>
Eengineer1
pushed a commit
to cheqd/cosmos-sdk
that referenced
this pull request
Aug 26, 2022
…0045) (cosmos#10269) * fix!: remove grpc query routing through tendermint (cosmos#10045) <!-- The default pull request template is for types feat, fix, or refactor. For other templates, add one of the following parameters to the url: - template=docs.md - template=other.md --> Revert routing queries through tendermint. The reason this change was made is because of this error: ``` fatal error: concurrent map read and map write ``` The person who identified this error submitted these steps to reproduce ``` User sends a query with grpc Tendermint Commit new blocks and tries to Commit IAVL (which causing IAVL versions map to change) At the same time query tries to read from the same map (iavl.(*MutableTree).VersionExists) to check if requested version is exists Node exits with fatal error: concurrent map read and map write ``` With the recent changes to IAVL submitted by terra (cc @YunSuk-Yeo) the reason for why we need to route through tendermint is no longer present. We should revert it when 0.17.1 of IAVL is cut, which will be later today. --- *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit 8091846) * fix conflicts * revert Co-authored-by: Marko <marbar3778@yahoo.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Revert routing queries through tendermint.
The reason this change was made is because of this error:
The person who identified this error submitted these steps to reproduce
With the recent changes to IAVL submitted by terra (cc @YunSuk-Yeo) the reason for why we need to route through tendermint is no longer present. We should revert it when 0.17.1 of IAVL is cut, which will be later today.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change