-
Notifications
You must be signed in to change notification settings - Fork 636
rpc: Add gRPC support #81
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
Comments
Some of the endpoints we should aim to provide in the first iteration of our API should at least provide the same functionality as those exposed by Penumbra's |
Partially addresses #81. [:book: Rendered](https://github.com/cometbft/cometbft/blob/thane/81-grpc-adr/docs/references/architecture/adr-106-grpc-api.md) There's already a partial implementation of this on the `feature/adr101-pull-companion` branch. See #818. --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Signed-off-by: Thane Thomson <connect@thanethomson.com> Co-authored-by: Andy Nogueira <me@andynogueira.dev>
…… (backport cometbft#76) (cometbft#81) * perf(blockstore): Add LRU caches to blockstore operations used in consensus (backport cometbft#3003) (cometbft#3083) Closes cometbft#2844 We are seeing that the blockstore loading operations get used in hot loops within gossip routines, and queryMaj23 routines. This PR reduces that overhead using an LRU cache. The LRU cache does have a mutex on every get, but the time with the LRU cache is 9x faster than without (before even adding in DB overheads), due to the proto unmarshalling saved. We could imagine a setup where we avoided a lock there entirely. I don't think this is worth right now, since the new code is 9x faster, and these mostly appear in catchup code which should not be highly contended for across peers at the same time. With the new benchmark I added: OLD: ``` BenchmarkRepeatedLoadSeenCommit-12 24447 54691 ns/op 46495 B/op 319 allocs/op ``` NEW: ``` BenchmarkRepeatedLoadSeenCommit-12 224131 6401 ns/op 8320 B/op 2 allocs/op ``` It turns out these gossip routines don't need mutative copies, so we could optimize out the large allocation in the future if we want. 1 hour cpu profile that shows this appearing in prod:  The state machine execution time here for context is 92 seconds. So this is adding up in system load (and GC! The GC load is 52GB, the entire trace is 200GB, with other parts being optimized down from recent PRs) --- - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments - [ ] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec <hr>This is an automatic backport of pull request cometbft#3003 done by [Mergify](https://mergify.com). --------- Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> (cherry picked from commit 0c10bd5) * Add Changelog (cherry picked from commit 4594f29) # Conflicts: # CHANGELOG.md --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Dev Ojha <dojha@berkeley.edu> Co-authored-by: PaddyMc <paddymchale@hotmail.com>
Hi @alesfor, do you want to take this one too? Please let me know, thanks |
If the work in #2693 is completed then this tracking issue can be closed. |
Uh oh!
There was an error while loading. Please reload this page.
At present, we support a flavor of JSON-RPC as one of the primary ways of integrating with a running node.
From discussions with users (e.g. @hdevalence), it's clear there's a need for a full gRPC interface to a node. This would dramatically simplify client generation for interaction with a node, and would help get around our very odd (inherited) JSON serialization quirks.
There's some context provided by ADR-057, but no clear decision was documented in that ADR. Previously, the team had decided to deprecate and remove gRPC support (see tendermint/tendermint#7121 and tendermint/tendermint#9683), but that decision has now been reversed.
Definition of Done
There are several deliverables associated with this issue, after which it could be considered done:
main
, we can consider whether we should backport this to older releases.Original issue: tendermint/tendermint#9751
The text was updated successfully, but these errors were encountered: