try-runtime-cli: Lazy Download

[kianenigma]

see: #13109

[liamaharon]

related: chopsticks storage layer https://github.com/AcalaNetwork/chopsticks/blob/master/packages/chopsticks/src/blockchain/storage-layer.ts

[ggwpez]

I tried out this subway to see if it could handle the batch request that you are currently fixing in one MR.
But it did not work. However, it is good for caching repeated queries.

[liamaharon]

I tried out this subway to see if it could handle the batch request that you are currently fixing in one MR. But it did not work. However, it is good for caching repeated queries.

This could be useful - but worth keeping in mind that (unless using a remote node) the biggest bottleneck is merkelizing the keys when loading them in TextExternalities, rather than fetching them from the node

[ggwpez]

Yes there could also be a much more optimized way for this; like having an export-state RPC or something that encodes and sends the whole Trie. But yea, the lazy approach should also help a lot.

[kianenigma]

Also, I think Centrifuge's fudge might have the right inspirational piece of code that lets you have Ext be backed by a database path: https://github.com/centrifuge/fudge

[xlc]

I tried out this subway to see if it could handle the batch request that you are currently fixing in one MR.
But it did not work. However, it is good for caching repeated queries.

batch request is not yet implemented but it is trivial to support it

[bkchr]

something that encodes and sends the whole Trie

This is state sync. This downloads the entire trie at a given block. It is used by warp sync.

Also, I think Centrifuge's fudge might have the right inspirational piece of code that lets you have Ext be backed by a database path: https://github.com/centrifuge/fudge

What you mean by backed by a database path? Ext is in "normal mode" always operating on a db, but I assume you want a db to cache your queries?

[liamaharon]

This is state sync. This downloads the entire trie at a given block. It is used by warp sync.

Thanks @bkchr this sounds very worth looking into. Perhaps not for lazy download but for saving entire state to disk that can quickly be loaded up again on subsequent runs (existing snapshots created by 'create-snapshot' still take 300+ seconds on my M2 Max Macbook Pro to load, bc building the tri is so expensive).

I've been meaning to create an issue to improve the performance of 'snapshot' mode, will mention this as an option to consider.

[xlc]

why does it need to build trie? there is (almost) no such logic in chopsticks

[liamaharon]

why does it need to build trie? there is (almost) no such logic in chopsticks

This is a good question. The trie may even be disadvantageous it'll slow things down a lot and it's unlikely that there'll be many keys needed cached in-memory at any one time.

I'll start hacking on a simple storage layer that accesses storage from a node and stashes results in a flat in-memory LRU cache. No trie, just flat key-value.

If this works I'll try different workloads on it and see how it performs both in terms of memory and speed.

[bkchr]

I'll start hacking on a simple storage layer that accesses storage from a node and stashes results in a flat in-memory LRU cache. No trie, just flat key-value.

You can just use the OverlayedChanges?

[arkpar]

(existing snapshots created by 'create-snapshot' still take 300+ seconds

If I'm reading the code correctly, this could be because key-values are inserted in the trie one at a time here:

substrate/primitives/state-machine/src/testing.rs

Line 137 in 2b91202

self.backend.insert(vec![(None, vec![(k, Some(v))])], self.state_version);

If you pass multiple/all values at once, it should be much faster because the trie root would not need to be recalculated after each insertion.

[liamaharon]

(existing snapshots created by 'create-snapshot' still take 300+ seconds

If I'm reading the code correctly, this could be because key-values are inserted in the trie one at a time here:

substrate/primitives/state-machine/src/testing.rs

Line 137 in 2b91202

self.backend.insert(vec![(None, vec![(k, Some(v))])], self.state_version);

If you pass multiple/all values at once, it should be much faster because the trie root would not need to be recalculated after each insertion.

I assumed they were being inserted one at a time for a reason. I'll try batch inserting, if it works it'll speed things up dramatically, thanks for the advice.

edit: this had a big impact, PR with before/after performance benchmarks: #14004

[kianenigma]

why does it need to build trie?

Ideally, we want the try-runtime-enabled runtime to be able to operate in a normal mode. In that case, one requirement would be to be able to respond to storage::root host function correctly, ergo using a trie.

If we remove this assumption, a whole lot of things will be easier, and as @liamaharon pointed in a comment, a flat key -> value map would suffice. We could achieve this by implementing trait Externalities for a new type that works mostly the same, is backed by HashMap, and panics if you ask it to calculate the state root. As @bkchr said, this might very well be just OverlayedChanges.

Ext is in "normal mode" always operating on a db, but I assume you want a db to cache your queries?

Not just for caching, but also as the first point of response. The scenario I have in mind is that you run try-runtime-cli with some sub-command, and instead of your state being an RPC endpoint or a snapshot file, you give the path to an existing (RocksDB/ParityDB) polkadot/substrate database, and it would initialize an Ext based on that, and execute the commands there.

[bkchr]

and panics if you ask it to calculate the state root

That isn't required, you can just take all the key/value pairs and calculate the storage root :P No problem.

Not just for caching, but also as the first point of response. The scenario I have in mind is that you run try-runtime-cli with some sub-command, and instead of your state being an RPC endpoint or a snapshot file, you give the path to an existing (RocksDB/ParityDB) polkadot/substrate database, and it would initialize an Ext based on that, and execute the commands there.

This is basically an enhanced OverlayedChanges. OverlayedChanges is already caching writes and then only goes to the state if the key is not yet written. Basically it should be enough for you to keep the OverlayedChanges changes around. This also already supports calculating a storage root etc.

[gpestana]

instead of your state being an RPC endpoint or a snapshot file, you give the path to an existing (RocksDB/ParityDB) polkadot/substrate database, and it would initialize an Ext based on that, and execute the commands there.

fwiw, ➕ for this approach. In the case of data gathering for analysis and block replays with different state for testing purposes this would be very useful.

[liamaharon]

instead of your state being an RPC endpoint or a snapshot file, you give the path to an existing (RocksDB/ParityDB) polkadot/substrate database, and it would initialize an Ext based on that, and execute the commands there.

fwiw, ➕ for this approach. In the case of data gathering for analysis and block replays with different state for testing purposes this would be very useful.

@gpestana are there cases where reading from a DB path would be helpful in ways that reading from a snapshot would not? Snapshots can be created from arbitrary block numbers and (shortly) will be able to be loaded almost instantly.

Update on this: I've had success writing directly to the ext OverlayedChanges (rather than backend) which allows for close to instant snapshot loading. WIP PR: #14057

[liamaharon]

FYI, I have created a dedicated issue to track the progress of allowing db-backed externalities to be used with the try-runtime-cli. Let's maintain the focus of this issue on lazy-download.

Upon introducing a DB path option, I think lazy-download is likely to be a preferred mode only when using a remote node and the state cannot fit in memory.

Node Location	State Size	Preferred Mode
local	reasonable	snapshot
remote	reasonable	snapshot
local	large	db-backed
remote	large	lazy-download

Although we should definitely eventually implement lazy-download, I wanted to note that it may not be as high priority as some other try-runtime-cli tasks, because most bases will be covered and I'm uncertain how prevalent use case of using it with a large remote node will be.

[crystalin]

Hi, I'm curious if there is currently some progress on it

[ggwpez]

I think its still a high value issue - just currently resource starved. We instead prioritized the versioned runtime upgrades to reduce the number of error sources when migrating.
Hopefully once that is done maybe @liamaharon (OOO) can pick it up or distribute to someone.

[liamaharon]

Yeah I've made decent progress with this and have a working PoC, I am planing to pick it up again once the higher prio runtime upgrade stuff is shipped.

@crystalin fyi I have actually run the PoC with moonbeam state and runtime testing a runtime migration and it executed in just a few seconds :)

[crystalin]

That's awesome.
I'll definitely help testing it

[liamaharon]

Hey @crystalin, we've decided to deprioritise this for now to focus on some higher priority features.

I have a PoC branch here you're welcome to pick up if you need the feature.

[liamaharon]

Moved: paritytech/try-runtime-cli#8