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 servers: add resource limiting. #47

Closed
niklasad1 opened this issue May 12, 2022 · 5 comments
Closed

RPC servers: add resource limiting. #47

niklasad1 opened this issue May 12, 2022 · 5 comments
Assignees
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I5-enhancement An additional feature request.

Comments

@niklasad1
Copy link
Member

niklasad1 commented May 12, 2022

As substrate has switched to jsonrpsee it provides resources limiting for RPC calls.
It should be used to reject calls that exceeds the maximum defined resources.

Requires profiling and to provide some base resources such as CPU, memory and similar.

@bkchr
Copy link
Member

bkchr commented May 12, 2022

@niklasad1 is that something you are going to work on or is this issue just for tracking this stuff?

@niklasad1
Copy link
Member Author

niklasad1 commented May 12, 2022

I haven't planned to work on this right now, it's a tracking issue so we don't forget about that.

So if anyone wants to work on this I'm happy to help mentoring this.

@bkchr
Copy link
Member

bkchr commented May 12, 2022

Maybe something for you @koute to work on as some "side project"?

@koute
Copy link
Contributor

koute commented May 13, 2022

Sure; sounds good to me.

We can probably start with one or two RPC endpoints and see how it goes.

I'll assign myself to this issue and take a stab at it in the near future.

@koute koute self-assigned this May 13, 2022
@the-right-joyce the-right-joyce transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I5-enhancement An additional feature request. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. and removed J0-enhancement labels Aug 25, 2023
@niklasad1
Copy link
Member Author

Superseeded by #3028

@github-project-automation github-project-automation bot moved this from backlog to done in SDK Node Jan 23, 2024
jonathanudd pushed a commit to jonathanudd/polkadot-sdk that referenced this issue Apr 10, 2024
github-merge-queue bot pushed a commit that referenced this issue May 28, 2024
**Don't look at the commit history, it's confusing, as this branch is
based on another branch that was merged**

Fixes #598 
Also implements [RFC
#47](polkadot-fellows/RFCs#47)

## Description

- Availability-recovery now first attempts to request the systematic
chunks for large POVs (which are the first ~n/3 chunks, which can
recover the full data without doing the costly reed-solomon decoding
process). This has a fallback of recovering from all chunks, if for some
reason the process fails. Additionally, backers are also used as a
backup for requesting the systematic chunks if the assigned validator is
not offering the chunk (each backer is only used for one systematic
chunk, to not overload them).
- Quite obviously, recovering from systematic chunks is much faster than
recovering from regular chunks (4000% faster as measured on my apple M2
Pro).
- Introduces a `ValidatorIndex` -> `ChunkIndex` mapping which is
different for every core, in order to avoid only querying the first n/3
validators over and over again in the same session. The mapping is the
one described in RFC 47.
- The mapping is feature-gated by the [NodeFeatures runtime
API](#2177) so that it
can only be enabled via a governance call once a sufficient majority of
validators have upgraded their client. If the feature is not enabled,
the mapping will be the identity mapping and backwards-compatibility
will be preserved.
- Adds a new chunk request protocol version (v2), which adds the
ChunkIndex to the response. This may or may not be checked against the
expected chunk index. For av-distribution and systematic recovery, this
will be checked, but for regular recovery, no. This is backwards
compatible. First, a v2 request is attempted. If that fails during
protocol negotiation, v1 is used.
- Systematic recovery is only attempted during approval-voting, where we
have easy access to the core_index. For disputes and collator
pov_recovery, regular chunk requests are used, just as before.

## Performance results

Some results from subsystem-bench:

with regular chunk recovery: CPU usage per block 39.82s
with recovery from backers: CPU usage per block 16.03s
with systematic recovery: CPU usage per block 19.07s

End-to-end results here:
#598 (comment)

#### TODO:

- [x] [RFC #47](polkadot-fellows/RFCs#47)
- [x] merge #2177 and
rebase on top of those changes
- [x] merge #2771 and
rebase
- [x] add tests
- [x] preliminary performance measure on Versi: see
#598 (comment)
- [x] Rewrite the implementer's guide documentation
- [x] #3065 
- [x] paritytech/zombienet#1705 and fix
zombienet tests
- [x] security audit
- [x] final versi test and performance measure

---------

Signed-off-by: alindima <alin@parity.io>
Co-authored-by: Javier Viola <javier@parity.io>
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this issue Jun 5, 2024
**Don't look at the commit history, it's confusing, as this branch is
based on another branch that was merged**

Fixes paritytech#598 
Also implements [RFC
paritytech#47](polkadot-fellows/RFCs#47)

## Description

- Availability-recovery now first attempts to request the systematic
chunks for large POVs (which are the first ~n/3 chunks, which can
recover the full data without doing the costly reed-solomon decoding
process). This has a fallback of recovering from all chunks, if for some
reason the process fails. Additionally, backers are also used as a
backup for requesting the systematic chunks if the assigned validator is
not offering the chunk (each backer is only used for one systematic
chunk, to not overload them).
- Quite obviously, recovering from systematic chunks is much faster than
recovering from regular chunks (4000% faster as measured on my apple M2
Pro).
- Introduces a `ValidatorIndex` -> `ChunkIndex` mapping which is
different for every core, in order to avoid only querying the first n/3
validators over and over again in the same session. The mapping is the
one described in RFC 47.
- The mapping is feature-gated by the [NodeFeatures runtime
API](paritytech#2177) so that it
can only be enabled via a governance call once a sufficient majority of
validators have upgraded their client. If the feature is not enabled,
the mapping will be the identity mapping and backwards-compatibility
will be preserved.
- Adds a new chunk request protocol version (v2), which adds the
ChunkIndex to the response. This may or may not be checked against the
expected chunk index. For av-distribution and systematic recovery, this
will be checked, but for regular recovery, no. This is backwards
compatible. First, a v2 request is attempted. If that fails during
protocol negotiation, v1 is used.
- Systematic recovery is only attempted during approval-voting, where we
have easy access to the core_index. For disputes and collator
pov_recovery, regular chunk requests are used, just as before.

## Performance results

Some results from subsystem-bench:

with regular chunk recovery: CPU usage per block 39.82s
with recovery from backers: CPU usage per block 16.03s
with systematic recovery: CPU usage per block 19.07s

End-to-end results here:
paritytech#598 (comment)

#### TODO:

- [x] [RFC paritytech#47](polkadot-fellows/RFCs#47)
- [x] merge paritytech#2177 and
rebase on top of those changes
- [x] merge paritytech#2771 and
rebase
- [x] add tests
- [x] preliminary performance measure on Versi: see
paritytech#598 (comment)
- [x] Rewrite the implementer's guide documentation
- [x] paritytech#3065 
- [x] paritytech/zombienet#1705 and fix
zombienet tests
- [x] security audit
- [x] final versi test and performance measure

---------

Signed-off-by: alindima <alin@parity.io>
Co-authored-by: Javier Viola <javier@parity.io>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this issue Aug 2, 2024
**Don't look at the commit history, it's confusing, as this branch is
based on another branch that was merged**

Fixes paritytech#598 
Also implements [RFC
paritytech#47](polkadot-fellows/RFCs#47)

## Description

- Availability-recovery now first attempts to request the systematic
chunks for large POVs (which are the first ~n/3 chunks, which can
recover the full data without doing the costly reed-solomon decoding
process). This has a fallback of recovering from all chunks, if for some
reason the process fails. Additionally, backers are also used as a
backup for requesting the systematic chunks if the assigned validator is
not offering the chunk (each backer is only used for one systematic
chunk, to not overload them).
- Quite obviously, recovering from systematic chunks is much faster than
recovering from regular chunks (4000% faster as measured on my apple M2
Pro).
- Introduces a `ValidatorIndex` -> `ChunkIndex` mapping which is
different for every core, in order to avoid only querying the first n/3
validators over and over again in the same session. The mapping is the
one described in RFC 47.
- The mapping is feature-gated by the [NodeFeatures runtime
API](paritytech#2177) so that it
can only be enabled via a governance call once a sufficient majority of
validators have upgraded their client. If the feature is not enabled,
the mapping will be the identity mapping and backwards-compatibility
will be preserved.
- Adds a new chunk request protocol version (v2), which adds the
ChunkIndex to the response. This may or may not be checked against the
expected chunk index. For av-distribution and systematic recovery, this
will be checked, but for regular recovery, no. This is backwards
compatible. First, a v2 request is attempted. If that fails during
protocol negotiation, v1 is used.
- Systematic recovery is only attempted during approval-voting, where we
have easy access to the core_index. For disputes and collator
pov_recovery, regular chunk requests are used, just as before.

## Performance results

Some results from subsystem-bench:

with regular chunk recovery: CPU usage per block 39.82s
with recovery from backers: CPU usage per block 16.03s
with systematic recovery: CPU usage per block 19.07s

End-to-end results here:
paritytech#598 (comment)

#### TODO:

- [x] [RFC paritytech#47](polkadot-fellows/RFCs#47)
- [x] merge paritytech#2177 and
rebase on top of those changes
- [x] merge paritytech#2771 and
rebase
- [x] add tests
- [x] preliminary performance measure on Versi: see
paritytech#598 (comment)
- [x] Rewrite the implementer's guide documentation
- [x] paritytech#3065 
- [x] paritytech/zombienet#1705 and fix
zombienet tests
- [x] security audit
- [x] final versi test and performance measure

---------

Signed-off-by: alindima <alin@parity.io>
Co-authored-by: Javier Viola <javier@parity.io>
liuchengxu added a commit to subcoin-project/polkadot-sdk that referenced this issue Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I5-enhancement An additional feature request.
Projects
Status: done
Development

No branches or pull requests

4 participants