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

chainHead: Soft limit for the number of pinned blocks #33

Closed
lexnv opened this issue Feb 17, 2023 · 2 comments · Fixed by #43
Closed

chainHead: Soft limit for the number of pinned blocks #33

lexnv opened this issue Feb 17, 2023 · 2 comments · Fixed by #43
Assignees

Comments

@lexnv
Copy link
Contributor

lexnv commented Feb 17, 2023

Would approx 64 pinned blocks suffice for all light-client interactions?

The spec is flexible regarding the number of pinned blocks and does not impose any hardcoded values:

The JSON-RPC server is strongly encouraged to enforce a limit to the maximum number of pinned blocks. If this limit is reached, it should then stop the subscription by emitting a stop event. This specification does not mention any specific limit, but it should be large enough for clients to be able to pin all existing non-finalized blocks and a few finalized blocks.

When integrating the pinning API into the chainhead methods, we have a limit of 1024 pinned blocks across all subscriptions, coming from substrate.

The short conclusion from paritytech/substrate#13233 (comment), we are considering:

  • soft limit of 64 pinned blocks per subscription (approx 6 minutes - considering all pinned blocks to be finalized)
  • a hard limit of 1024 (all available blocks)

After the soft limit is exceeded for a subscription, the subscription could get terminated (via stop event) at any time.

Another idea would be to allow a soft limit of 64 finalized pinned blocks and a soft limit of 128 total blocks; this way we can guarantee ~6 minutes of blocks.

We could even reduce the limits to 32 finalized pinned blocks; and 64 total. I do remember when we considered the "delay pruning" solution paritytech/substrate#12497 that 32 blocks could be appropriate for a first iteration.

If that would suffice for all use cases, we could add something similar in the spec to mention the soft limit, while still keeping other implementations of the spec flexible:

The substrate JSON-RPC server guarantees 64 finalized pinned blocks and a total of 128 pinned blocks including forks per subscription. After the limit is exceeded, the subscription could be terminated at any time. The user is strongly encouraged to stay within this soft limit. If you need access to older blocks, please use the archive methods.

Would love to hear your thoughts on this!

// CC: @paritytech/tools-team @tomaka @bkchr @skunert @jsdw

@lexnv lexnv self-assigned this Feb 17, 2023
@tomaka
Copy link
Contributor

tomaka commented Feb 20, 2023

Another idea would be to allow a soft limit of 64 finalized pinned blocks and a soft limit of 128 total blocks; this way we can guarantee ~6 minutes of blocks.

That's not good IMO. If finality stalls for more than 64 blocks, then anyone who subscribes will get their subscription immediately cancelled. In other words, the JSON-RPC server will basically completely stop working if finality stalls for ~10 minutes, which has happened many times in the past.

Even 1024 blocks seems too little, as it represents ~3 hours of blocks. It could totally be possible that finality stalls for 3 hours in case of a bug or netsplit.

In smoldot, there is simply no limit for the number of total blocks. There is only a limit for the number of pinned finalized blocks.
There are several areas of the node that simply must assume that finality works, and where having a total limit would be wrong. Finality is a bit the garbage collector of blocks, and there's no way around that fact.

@tomaka
Copy link
Contributor

tomaka commented Feb 20, 2023

As for the limit of finalized blocks (the "soft" limit, which IMO should be the only limit), it can indeed be quite low.

In general, the API user is expected to keep the current finalized block pinned, but not any block below.

The reason why you still need to give the possibility to have blocks below the current finalized one pinned for a bit of time due to race conditions. If the API user is for example querying multiple storage items from the current finalized block, and the next block gets finalized, you want the client to be gracefully made aware of this so that it can finish what it's doing and use the next block instead. You don't want it to be immediately shut off.

One problem with having that limit as a number of blocks is that it is possible for dozens of blocks to be finalized at once (in case of bug or netsplit), but that's rare enough that sending a stop event in that case shouldn't be problematic.
But maybe a different way of specifying that limit is that you are allowed to keep a block pinned for N seconds or minutes after it has been reported as finalized.

Overall, I think that there is no obvious best way to specify that limit, but also that it's not worth spending dozens of hours debating how to define a precise limit, because in practice from the server's point of view it really doesn't matter whether the limit is 16, 32, or 64 for example.

In conclusion, I think that the spec should simply say that clients shouldn't keep blocks pinned for "too long" (one minute-ish) after they are no longer the current finalized block (in other words, block N should be unpinned relatively quickly when N+1 is finalized), and that servers can send a stop if it thinks that clients are taking too long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants