[Forknet V2] Aggressive database trimmer that works with memtrie. #11420
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11420 +/- ##
==========================================
- Coverage 71.44% 71.19% -0.26%
==========================================
Files 785 789 +4
Lines 158765 159645 +880
Branches 158765 159645 +880
==========================================
+ Hits 113432 113656 +224
- Misses 40436 41047 +611
- Partials 4897 4942 +45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
robin-near
changed the title
robin-near
force-pushed
the
delete3
branch
2 times, most recently
from
4296581
to
a3c0caa
Compare
robin-near
requested review from
tayfunelmas,
Longarithm and
marcelo-gonzalez
and removed request for
Longarithm
There was a problem hiding this comment.
overall this looks pretty sane to me, but sadly doesnt seem to work right now on localnet... @robin-near let me know if you have trouble reproducing w the instructions i posted in that linked comment and i can give more details. I'm not sure if this is just some quirk of localnet pytests that makes something break here, but it looks like something sort of simple going wrong that shouldnt be too bad to fix hopefully
| }) | ||
| .collect::<Vec<_>>(); | ||
| split_points.sort(); | ||
| split_points.remove(0); // skip first split point |
There was a problem hiding this comment.
nit: could also just start iterating from 1 below instead of this O(n) operation? but if i understand correctly this will have around 256 elements so doesnt matter too much?
There was a problem hiding this comment.
I'm afraid of a crash though, if the whole range happens to be the empty prefix. I can also start iterating from 1 but it makes the code feel less easy to see that it's safe.
The O(n) operation is trivial anyway because at most this will have about a couple of thousand elements.
| } | ||
| important_keys | ||
| }); | ||
| for shard_uid in shard_uids { |
There was a problem hiding this comment.
not a big deal to do this now, but do we also want to parallelize this part? we might wait for a while working on a shard that doesnt fully utilize all cores, when we could be making progress on other shards too
There was a problem hiding this comment.
It wouldn't be of much help. This part is negligible compared to the copying-over-of-flat-state part. In practice it'll take at most a few seconds each, whereas the copying will take a few minutes.
| let temp_store = temp_storage.get_hot_store(); | ||
| let temp_store_path = temp_store_home.join("data"); | ||
|
|
||
| trim_database::trim_database(store, &near_config.genesis.config, temp_store)?; |
There was a problem hiding this comment.
I'm getting this error:
thread 'main' panicked at tools/fork-network/src/cli.rs:148:88:
called `Result::unwrap()` on an `Err` value: MissingTrieValue(TrieStorage, 11111111111111111111111111111111)
to reproduce, apply the diff and run the two commands I posted in this comment: #11310 (comment)
I'm not sure exactly what is happening, but the error is in the call to prepare_memtrie_state_trimming() in trim_database()
There was a problem hiding this comment.
Oh. I think I know what happened, let me fix that.
There was a problem hiding this comment.
Hmm I have some trouble running the commands and I'm not sure if I did something wrong or something else, but I uploaded a fix that I think addresses the issue (handling empty tries); do you mind trying again?
There was a problem hiding this comment.
ah now i'm getting this:
thread 'main' panicked at tools/fork-network/src/cli.rs:148:88:
called `Result::unwrap()` on an `Err` value: MissingTrieValue(TrieStorage, 6Mswb4EnCzycL7n6bMEGaE87nnEboTj7K2BS3GyJjZbS
actually I dont know why I recommended doing the whole thing with the local mocknet stuff. You can reproduce this by just first running python3 tests/sanity/transactions.py
and then after that test is done, write something like this to /tmp/validators.json:
[{"account_id": "node0", "public_key": "ed25519:sqYjRhP3P1vzYY2nA7w2hCTZoA8hknuXpSeaReRxTKf", "amount": "1000000000000000000000000000000000"}, {"account_id": "node1", "public_key": "ed25519:4WFsnnwUj69smErNuidUEfeJs3fpHCQUJNsYzeghQVyT", "amount": "1000000000000000000000000000000000"}]
then just run the sequence of fork-network commands:
neard --home ~/.near/test4_finished fork-network init
neard --home ~/.near/test4_finished fork-network amend-access-keys
neard --home ~/.near/test4_finished fork-network set-validators --validators /tmp/validators.json
neard --home ~/.near/test4_finished fork-network finalize
|
still getting a crash like i posted in the other comment :(. also one thing I just realized, do we really want to make this the default thing that happens with |
Closes #11280
A rewrite of #11310
Includes two new commands:
neard database aggressive-trimming --obliterate-disk-trieperforms the deletion of all State keys except those that are needed for memtrie operation (large values not inlined by flat storage, as well as some trie nodes near the root). This is used to test that memtrie-only nodes can work.neard fork-network finalizenow rebuilds the database by copying out FlatState as well as the aforementioned necessary State keys, and then swapping in the new database replacing the old.