-
Notifications
You must be signed in to change notification settings - Fork 110
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
fix(blocktree): stop and prune runtimes from memory #3071
Conversation
752da95
to
b6c9f22
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## development #3071 +/- ##
===============================================
+ Coverage 51.41% 51.78% +0.36%
===============================================
Files 221 219 -2
Lines 28296 28041 -255
===============================================
- Hits 14549 14521 -28
+ Misses 12427 12232 -195
+ Partials 1320 1288 -32 |
b6c9f22
to
9093e0e
Compare
6f29f41
to
ea62f68
Compare
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.
Ran this with on westend chain. I see size of bt.runtime
getting increased as blocks get imported. imported a good number of blocks, but it doesn't looks like runtimes are getting pruned
number of runtimes stored: 26238
2023-02-10T19:18:53+05:30 DEBUG imported block 0xc179598e8992f82d9bf88afb690d8e7f8e65d002a3e336c87f980cc7c1c4cd50 and stored state trie with root 0x5a2ac0194ba3f39789bd212ebf3cee4bd6398bdde1d7223f5e54988a1f7b6d36 service.go:L212 pkg=core
number of runtimes stored: 26238
2023-02-10T19:18:53+05:30 DEBUG 🔗 imported block number 111742 with hash 0xc179598e8992f82d9bf88afb690d8e7f8e65d002a3e336c87f980cc7c1c4cd50 chain_processor.go:L272 pkg=sync
2023-02-10T19:18:53+05:30 DEBUG block with hash 0xc179598e8992f82d9bf88afb690d8e7f8e65d002a3e336c87f980cc7c1c4cd50 processed chain_processor.go:L144 pkg=sync
2023-02-10T19:18:53+05:30 DEBUG processing block data with hash 0xe24646cd5c86b20a5c1c62d0e48f077019409f8feba9458fdfab947c79b0ef4b chain_processor.go:L115 pkg=sync
number of runtimes stored: 26239
2023-02-10T19:18:53+05:30 DEBUG imported block 0xe24646cd5c86b20a5c1c62d0e48f077019409f8feba9458fdfab947c79b0ef4b and stored state trie with root 0x2fb5be18eadcbdbeddaf57c4103b0f48ff3c4346d1381344428e11d3609a7e98 service.go:L212 pkg=core
number of runtimes stored: 26239
2023-02-10T19:18:53+05:30 DEBUG 🔗 imported block number 111743 with hash 0xe24646cd5c86b20a5c1c62d0e48f077019409f8feba9458fdfab947c79b0ef4b chain_processor.go:L272 pkg=sync
2023-02-10T19:18:53+05:30 DEBUG block with hash 0xe24646cd5c86b20a5c1c62d0e48f077019409f8feba9458fdfab947c79b0ef4b processed chain_processor.go:L144 pkg=sync
2023-02-10T19:18:53+05:30 DEBUG processing block data with hash 0x1d5f85989e3ca6afc12f30434877a9646879496780527a9ce3e5cdd5046c9da2 chain_processor.go:L115 pkg=sync
number of runtimes stored: 26240
I am printing following in (bt *BlockTree) GetBlockRuntime
fmt.Printf("number of runtimes stored: %d\n", len(bt.runtimes.mapping))
ea62f68
to
6844597
Compare
6844597
to
5398c36
Compare
This should now work, and it has tests with it. @kishansagathiya can you try running it until there is a runtime upgrade? Pruning of the runtimes mapping and stopping of runtime(s) only happen after a runtime upgrade, so make sure such upgrade happens before checking the mapping gets trimmed down. I gotta run and didn't have time to sync for more than a few thousands blocks without runtime upgrade. There is |
@qdm12 I will take a look on it |
I am not sure at what block do I see the first runtime. So far I have ran it till only 91k blocks and number of saves runtimes are also 91k. This still stores a lot of runtimes in memory that we don't need. Can we not remove them before runtime upgrade or store them cleverly such that it takes less memory? |
@qdm12 The test Related to:
I think in the production environment when we finalize a block I don't think we ensure that exists an entry in the runtimes mapping, so it is possible that for a new finalized block a mapping entry could be not there. The test was crashing because while adding blocks to the block tree we don't store runtimes so, within the test, when we were marking some blocks as finalized we face panic. Let me know if this is not what you were asking |
Sorry for the delayed response;
It only stores the mapping block hash <-> runtime pointer, so not a big deal. And we need this mapping, ideally from memory since we lookup a lot from it. It doesn't store a separate runtime instance for each entry. So really per entry it's 32B (block hash) + 8B (runtime pointer) + map overhead, so even a few million entries is fine. That was discussed previously (I think at standup?) if it was okay to keep this map growing, especially if we have no runtime upgrade for millions of blocks. I think we can address having a more performant system (maybe on disk + lru cache) once we encounter a problem later down the line (we might never do at least on westend/kusama/polkadot).
This is the case. We only have one instantiated runtime from the last finalised block, and may have more instantiated runtimes until the next finalisation (i.e. 2 more runtimes if we do a runtime upgrade on 2 forks).
Are you sure? Don't we add the mapping for every handled block?? |
0dc9581
to
dfe2578
Compare
Changes
Prune
methodNote this PR breaks RPC calls
payment_Queryinfo
,runtime_getVersion
andruntime_getMetadata
which no longer have access to historical runtimes.Tests
go test -tags integration github.com/ChainSafe/gossamer/lib/blocktree
Issues
Fixes #2767
Sibling issue to fix next is #3066 to re-establish the 3 RPC calls working for blocks older than the last finalised block.
Primary Reviewer
@timwu20