-
Notifications
You must be signed in to change notification settings - Fork 654
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
feat: increase main_storage_proof_size_soft_limit #11997
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11997 +/- ##
==========================================
- Coverage 71.53% 71.52% -0.02%
==========================================
Files 814 814
Lines 164424 164425 +1
Branches 164424 164425 +1
==========================================
- Hits 117623 117597 -26
- Misses 41640 41662 +22
- Partials 5161 5166 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There's an integration test for soft size limits, I was surprised that it didn't fail:
nearcore/integration-tests/src/tests/client/features/storage_proof_size_limit.rs
Line 131 in 51e4728
// Spawn 3 transactions, each reading 2MB of data. The first two should end up in the same chunk. |
I see that it submits multiple receipts that read 2MB of state and expects the limit to be hit after 2 receipts are executed. That probably didn't change when the limit changed, with a 4MB limit it might also stop processing things after two receipts. It could be good to make the test more sensistive, maybe submit receipts that read 100kB of state instead of 2MB?
@jancionear thanks for pointing out the test, it actually helped me to discover that I've forgotten to update stable version: 9b94e67 |
Ah right, missed that too. You'll also probably need to update the hashes generated by |
Aug 30th status report
|
This PR changes storage proof soft size limit from 3mb to 4mb. Soft limits defines the value after which we stop executing receipts and move them to the delayed queue. This change addresses the issues on mainnet where we have up to 12% of chunks hitting the size limit. It is worse on shard 5 where it reaches 25%. With this PR we decrease the number of such chunks by ~70%. This change potentially increases the max possible size of state witness, but in practice compressed storage proof doesn't increase much with the current mainnet traffic (`experiment` is 4mb limit and `update_shard` is 3mb): <img width="1116" alt="Screenshot 2024-08-29 at 13 48 00" src="https://github.com/user-attachments/assets/6d8dca33-aa39-469d-ad90-42fd3c83038b"> Also see [zulip thread](https://near.zulipchat.com/#narrow/stream/295306-contract-runtime/topic/increasing.20state.20witness.20storage.20proof.20soft.20size.20limit/near/465817493) for more context.
Update docs to reflect #11997.
This PR changes storage proof soft size limit from 3mb to 4mb. Soft limits defines the value after which we stop executing receipts and move them to the delayed queue.
This change addresses the issues on mainnet where we have up to 12% of chunks hitting the size limit. It is worse on shard 5 where it reaches 25%. With this PR we decrease the number of such chunks by ~70%.
This change potentially increases the max possible size of state witness, but in practice compressed storage proof doesn't increase much with the current mainnet traffic (
experiment
is 4mb limit andupdate_shard
is 3mb):Also see zulip thread for more context.