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

rpc: backpressured RPC server (bump jsonrpsee 0.20) #1313

Merged
merged 28 commits into from
Jan 23, 2024

Conversation

niklasad1
Copy link
Member

@niklasad1 niklasad1 commented Aug 30, 2023

This is a rather big change in jsonrpsee, the major things in this bump are:

  • Server backpressure (the subscription impls are modified to deal with that)
  • Allow custom error types / return types (remove jsonrpsee::core::Error and jsonrpee::core::CallError)
  • Bug fixes (graceful shutdown in particular not used by substrate anyway)
  • Less dependencies for the clients in particular
  • Return type requires Clone in method call responses
  • Moved to tokio channels
  • Async subscription API (not used in this PR)

Major changes in this PR:

  • The subscriptions are now bounded and if subscription can't keep up with the server it is dropped
  • CLI: add parameter to configure the jsonrpc server bounded message buffer (default is 64)
  • Add our own subscription helper to deal with the unbounded streams in substrate

The most important things in this PR to review is the added helpers functions in substrate/client/rpc/src/utils.rs and the rest is pretty much chore.

Regarding the "bounded buffer limit" it may cause the server to handle the JSON-RPC calls
slower than before.

The message size limit is bounded by "--rpc-response-size" thus "by default 10MB * 64 = 640MB"
but the subscription message size is not covered by this limit and could be capped as well.

Hopefully the last release prior to 1.0, sorry in advance for a big PR

Previous attempt: paritytech/substrate#13992

Resolves #748, resolves #627

@niklasad1 niklasad1 force-pushed the na-jsonrpsee-v0.20 branch 3 times, most recently from 590b12b to ad4d23e Compare August 30, 2023 17:35
@niklasad1 niklasad1 force-pushed the na-jsonrpsee-v0.20 branch 6 times, most recently from 52b35cf to ec7b24c Compare September 23, 2023 18:23
@niklasad1 niklasad1 force-pushed the na-jsonrpsee-v0.20 branch 3 times, most recently from 53af167 to a92968d Compare September 25, 2023 14:55
@nazar-pc
Copy link
Contributor

I'm following PR to see when it lands and I find it annoying how many notifications about force pushes just to fix clippy are happening. Can't you run clippy locally and fix it all at once before pushing to GitHub in the first place?

@niklasad1
Copy link
Member Author

niklasad1 commented Sep 25, 2023

Sorry about that but I'm force pushing to make it easy to backport to a previous polkadot release, I haven't fixed the tests when I rebased/merged it to master. That's why clippy complains ^^

Would be nice if Github could just subscribe you when it's actually PR and not draft 🙏

@niklasad1 niklasad1 force-pushed the na-jsonrpsee-v0.20 branch 2 times, most recently from 095d44e to 59a74ad Compare September 28, 2023 11:09
@niklasad1 niklasad1 force-pushed the na-jsonrpsee-v0.20 branch 3 times, most recently from 3896a14 to 0bcd8b0 Compare October 14, 2023 09:25
@niklasad1 niklasad1 force-pushed the na-jsonrpsee-v0.20 branch 2 times, most recently from 0f253e5 to c0acd5d Compare October 25, 2023 17:33
@niklasad1 niklasad1 force-pushed the na-jsonrpsee-v0.20 branch 2 times, most recently from c9a43f6 to d5d2d9b Compare October 30, 2023 09:49
@niklasad1 niklasad1 added the T0-node This PR/Issue is related to the topic “node”. label Oct 30, 2023
@niklasad1 niklasad1 changed the title WIP: jsonrpsee v0.20 rpc: backpressured RPC server (bump jsonrpsee 0.20) Oct 30, 2023
@@ -108,9 +108,9 @@ impl RpcHandlers {
pub async fn rpc_query(
&self,
json_query: &str,
) -> Result<(String, mpsc::UnboundedReceiver<String>), JsonRpseeError> {
) -> Result<(String, tokio::sync::mpsc::Receiver<String>), JsonRpseeError> {
Copy link
Member Author

@niklasad1 niklasad1 Oct 30, 2023

Choose a reason for hiding this comment

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

breaking change: bounded channel now

Not sure whether this should be regarded as a breaking change in node release, probably not as it's a custom API.

log = "0.4.17"
serde_json = "1.0.107"
tokio = { version = "1.22.0", features = ["parking_lot"] }
prometheus-endpoint = { package = "substrate-prometheus-endpoint", path = "../../utils/prometheus" }
tower-http = { version = "0.4.0", features = ["cors"] }
tower = "0.4.13"
tower = { version = "0.4.13", features = ["util"] }
Copy link
Member Author

@niklasad1 niklasad1 Oct 30, 2023

Choose a reason for hiding this comment

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

"feature = util" needed for the option_layer API.

@niklasad1
Copy link
Member Author

niklasad1 commented Jan 22, 2024

@kayabaNerve thanks, I changed the ping now :)

Copy link
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

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

Only some nits/questions, overall LGTM!

substrate/client/rpc/src/utils.rs Outdated Show resolved Hide resolved
substrate/client/rpc/src/utils.rs Show resolved Hide resolved
substrate/client/service/src/lib.rs Outdated Show resolved Hide resolved
@niklasad1 niklasad1 added this pull request to the merge queue Jan 23, 2024
Merged via the queue into master with commit e16ef08 Jan 23, 2024
123 of 124 checks passed
@niklasad1 niklasad1 deleted the na-jsonrpsee-v0.20 branch January 23, 2024 09:36
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
This is a rather big change in jsonrpsee, the major things in this bump
are:
- Server backpressure (the subscription impls are modified to deal with
that)
- Allow custom error types / return types (remove jsonrpsee::core::Error
and jsonrpee::core::CallError)
- Bug fixes (graceful shutdown in particular not used by substrate
anyway)
   - Less dependencies for the clients in particular
   - Return type requires Clone in method call responses
   - Moved to tokio channels
   - Async subscription API (not used in this PR)

Major changes in this PR:
- The subscriptions are now bounded and if subscription can't keep up
with the server it is dropped
- CLI: add parameter to configure the jsonrpc server bounded message
buffer (default is 64)
- Add our own subscription helper to deal with the unbounded streams in
substrate

The most important things in this PR to review is the added helpers
functions in `substrate/client/rpc/src/utils.rs` and the rest is pretty
much chore.

Regarding the "bounded buffer limit" it may cause the server to handle
the JSON-RPC calls
slower than before.

The message size limit is bounded by "--rpc-response-size" thus "by
default 10MB * 64 = 640MB"
but the subscription message size is not covered by this limit and could
be capped as well.

Hopefully the last release prior to 1.0, sorry in advance for a big PR

Previous attempt: paritytech/substrate#13992

Resolves paritytech#748, resolves
paritytech#627
AurevoirXavier added a commit to darwinia-network/darwinia that referenced this pull request Jun 24, 2024
AurevoirXavier added a commit to darwinia-network/darwinia that referenced this pull request Jun 28, 2024
* Setup deps

* Remove Koi from account migration test

* paritytech/polkadot-sdk#1495

* Bump

* paritytech/polkadot-sdk#1524

* !! paritytech/polkadot-sdk#1363

* paritytech/polkadot-sdk#1492

* paritytech/polkadot-sdk#1911

* paritytech/polkadot-sdk#1900

Signed-off-by: Xavier Lau <[email protected]>

* paritytech/polkadot-sdk#1661

* paritytech/polkadot-sdk#2144

* paritytech/polkadot-sdk#2048

* paritytech/polkadot-sdk#1672

* paritytech/polkadot-sdk#2303

* paritytech/polkadot-sdk#1256

* Remove identity and vesting

* Fixes

* paritytech/polkadot-sdk#2657

* paritytech/polkadot-sdk#1313

* paritytech/polkadot-sdk#2331

* paritytech/polkadot-sdk#2409 part.1

* paritytech/polkadot-sdk#2767

* paritytech/polkadot-sdk#2521

Signed-off-by: Xavier Lau <[email protected]>

* paritytech/polkadot-sdk#1222

* paritytech/polkadot-sdk#1234 part.1

* Satisfy compiler

* XCM V4 part.1

* paritytech/polkadot-sdk#1246

* Remove pallet-democracy part.1

* paritytech/polkadot-sdk#2142

* paritytech/polkadot-sdk#2428

* paritytech/polkadot-sdk#3228

* XCM V4 part.2

* Bump

* Build all runtimes

* Build node

* Remove pallet-democracy

Signed-off-by: Xavier Lau <[email protected]>

* Format

* Fix pallet tests

* Fix precompile tests

* Format

* Fixes

* Async, remove council, common pallet config

* Fix `ethtx-forward` test case (#1519)

* Fix ethtx-forward tests

* Format

* Fix following the review

* Fixes

* Fixes

* Use default impl

* Benchmark helper

* Bench part.1

* Bench part.2

* Bench part.3

* Fix all tests

* Typo

* Feat

* Fix EVM tracing build

* Reuse upstream `proof_size_base_cost()` (#1521)

* Format issue

* Fixes

* Fix CI

---------

Signed-off-by: Xavier Lau <[email protected]>
Co-authored-by: Bear Wang <[email protected]>
enddynayn added a commit to frequency-chain/frequency that referenced this pull request Jul 10, 2024
- Upgrade Polkadot-sdk to v.1.7.0.
- Update weights to reflect the new version.

Notable Changes:
- [Allow custom error types in Jsonrpsee](paritytech/polkadot-sdk#1313)

For more details, please refer to:

[Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.7.0)

issue-1870
enddynayn added a commit to frequency-chain/frequency that referenced this pull request Jul 10, 2024
- Upgrade Polkadot-sdk to v.1.7.0.
- Update weights to reflect the new version.

Notable Changes:
- [Allow custom error types in Jsonrpsee](paritytech/polkadot-sdk#1313)

For more details, please refer to:

[Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.7.0)

issue-1870
enddynayn added a commit to frequency-chain/frequency that referenced this pull request Jul 10, 2024
- Upgrade Polkadot-sdk to v.1.7.0.
- Update weights to reflect the new version.

Notable Changes:
- [Allow custom error types in Jsonrpsee](paritytech/polkadot-sdk#1313)

For more details, please refer to:

[Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.7.0)

issue-1870
enddynayn added a commit to frequency-chain/frequency that referenced this pull request Jul 10, 2024
- Upgrade Polkadot-sdk to v.1.7.0.
- Update weights to reflect the new version.

Notable Changes:
- [Allow custom error types in Jsonrpsee](paritytech/polkadot-sdk#1313)

For more details, please refer to:

[Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.7.0)

issue-1870
enddynayn added a commit to frequency-chain/frequency that referenced this pull request Jul 11, 2024
- Upgrade Polkadot-sdk to v.1.7.0.
- Update weights to reflect the new version.

Notable Changes:
- [Allow custom error types in Jsonrpsee](paritytech/polkadot-sdk#1313)

For more details, please refer to:

[Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.7.0)

issue-1870
enddynayn added a commit to frequency-chain/frequency that referenced this pull request Jul 11, 2024
- Upgrade Polkadot-sdk to v.1.7.0.
- Update weights to reflect the new version.

Notable Changes:
- [Allow custom error types in Jsonrpsee](paritytech/polkadot-sdk#1313)

For more details, please refer to:

[Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.7.0)

issue-1870
enddynayn added a commit to frequency-chain/frequency that referenced this pull request Jul 11, 2024
- Upgrade Polkadot-sdk to v.1.7.0.
- Update weights to reflect the new version.

Notable Changes:
- [Allow custom error types in Jsonrpsee](paritytech/polkadot-sdk#1313)

For more details, please refer to:

[Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.7.0)

issue-1870
enddynayn added a commit to frequency-chain/frequency that referenced this pull request Jul 11, 2024
- Upgrade Polkadot-sdk to v.1.7.0.
- Update weights to reflect the new version.

Notable Changes:
- [Allow custom error types in Jsonrpsee](paritytech/polkadot-sdk#1313)

For more details, please refer to:

[Release Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.7.0)

issue-1870
enddynayn added a commit to frequency-chain/frequency that referenced this pull request Jul 12, 2024
- Upgrade Polkadot-sdk to v.1.7.0.
- Update weights to reflect the new version.

Notable Changes:
- [Allow custom error types in
Jsonrpsee](paritytech/polkadot-sdk#1313)

For more details, please refer to:

[Release
Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.7.0)

#1870
rustadot pushed a commit to rustadot/recurrency that referenced this pull request Sep 5, 2024
- Upgrade Polkadot-sdk to v.1.7.0.
- Update weights to reflect the new version.

Notable Changes:
- [Allow custom error types in
Jsonrpsee](paritytech/polkadot-sdk#1313)

For more details, please refer to:

[Release
Notes](https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-v1.7.0)

#1870
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase in error rate after upgrade to v0.9.36 HTTP serve connection failed hyper::Error(Parse(Method))
6 participants