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

abci/grpc: return async responses in order #5520

Merged
merged 6 commits into from Oct 20, 2020
Merged

Conversation

erikgrinaker
Copy link
Contributor

@erikgrinaker erikgrinaker commented Oct 16, 2020

Fixes #5439. This is really a workaround for #5519 (unless we require async implementations to return ordered responses, but that kind of defeats the purpose of having an async API).

@erikgrinaker erikgrinaker added T:bug Type Bug (Confirmed) C:abci Component: Application Blockchain Interface C:mempool Component: Mempool labels Oct 16, 2020
@erikgrinaker erikgrinaker self-assigned this Oct 16, 2020
@codecov
Copy link

codecov bot commented Oct 16, 2020

Codecov Report

Merging #5520 into master will decrease coverage by 0.17%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #5520      +/-   ##
==========================================
- Coverage   61.26%   61.09%   -0.18%     
==========================================
  Files         263      263              
  Lines       23687    23694       +7     
==========================================
- Hits        14512    14476      -36     
- Misses       7703     7743      +40     
- Partials     1472     1475       +3     
Impacted Files Coverage Δ
abci/client/grpc_client.go 0.00% <0.00%> (ø)
crypto/sr25519/pubkey.go 43.47% <0.00%> (-8.70%) ⬇️
blockchain/v0/reactor.go 60.59% <0.00%> (-6.41%) ⬇️
privval/signer_endpoint.go 78.78% <0.00%> (-3.04%) ⬇️
p2p/switch.go 63.66% <0.00%> (-2.18%) ⬇️
consensus/state.go 67.97% <0.00%> (-0.94%) ⬇️
consensus/reactor.go 76.22% <0.00%> (-0.89%) ⬇️
mempool/reactor.go 84.09% <0.00%> (-0.76%) ⬇️
proxy/multi_app_conn.go 48.64% <0.00%> (ø)
p2p/pex/pex_reactor.go 79.46% <0.00%> (+0.59%) ⬆️
... and 1 more

@erikgrinaker erikgrinaker added the S:automerge Automatically merge PR when requirements pass label Oct 16, 2020
abci/client/grpc_client.go Outdated Show resolved Hide resolved
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

@mergify mergify bot merged commit 047267b into master Oct 20, 2020
@mergify mergify bot deleted the erik/abci-grpc-panics branch October 20, 2020 05:53
mergify bot pushed a commit that referenced this pull request Oct 20, 2020
Once #5520 lands, we can re-enable gRPC ABCI protocol in the E2E testnets.
erikgrinaker added a commit that referenced this pull request Oct 20, 2020
Fixes #5439. This is really a workaround for #5519 (unless we require async implementations to return ordered responses, but that kind of defeats the purpose of having an async API).
erikgrinaker added a commit that referenced this pull request Oct 20, 2020
Fixes #5439. This is really a workaround for #5519 (unless we require async implementations to return ordered responses, but that kind of defeats the purpose of having an async API).
erikgrinaker added a commit that referenced this pull request Oct 20, 2020
Once #5520 lands, we can re-enable gRPC ABCI protocol in the E2E testnets.
erikgrinaker added a commit that referenced this pull request Oct 22, 2020
Once #5520 lands, we can re-enable gRPC ABCI protocol in the E2E testnets.
erikgrinaker added a commit that referenced this pull request Oct 22, 2020
Once #5520 lands, we can re-enable gRPC ABCI protocol in the E2E testnets.
erikgrinaker added a commit that referenced this pull request Oct 22, 2020
Once #5520 lands, we can re-enable gRPC ABCI protocol in the E2E testnets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:abci Component: Application Blockchain Interface C:mempool Component: Mempool S:automerge Automatically merge PR when requirements pass T:bug Type Bug (Confirmed)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

abci/grpc: panic during concurrent CheckTx
2 participants