fix(store): reset_data_pre_state_sync memory usage #3988
Conversation
Add delete_all to StoreUpdate and implement it as a rocksdb range delete. Use it in reset_data_pre_state_sync to clear ColState. Test plan --------- unit test existing tests
mikhailOK
requested review from
evgenykuzyakov,
frol,
MaksymZavershynskyi and
SkidanovAlex
as code owners
| for shard_id in 0..self.caches.len() { | ||
| self.caches[shard_id].clear(); | ||
| } |
There was a problem hiding this comment.
Are there any tests checking cache clearing / discrepancy between cache and column?
There was a problem hiding this comment.
A discrepancy manifests itself as StorageInconsistentState: the invariant is that if a trie node is in cache+storage, then its children have to be there too, and forgetting to clear cache can break it.
At the moment ViewClient has its own cache and allows the error, and regular Client panics if it happens.
There was a problem hiding this comment.
what was the reason that we allow the error in view client (instead of cleaning the cache)?
There was a problem hiding this comment.
Tracked by #3742. Two reasons:
- Invalidating the cache requires taking a lock and we don't want ViewClient to block Client
- The error is theoretically possible in ViewClient because it doesn't read from snapshots. Client is single threaded and the only one who can write to storage, so it doesn't have that problem. If we ever decide to use snapshots we can bring back feat(client): Read snapshots for state #3092.
|
|
| for shard_id in 0..self.caches.len() { | ||
| self.caches[shard_id].clear(); | ||
| } |
There was a problem hiding this comment.
what was the reason that we allow the error in view client (instead of cleaning the cache)?
| let cf_handle = unsafe { &*self.cfs[col as usize] }; | ||
| let opt_first = self.db.iterator_cf(cf_handle, IteratorMode::Start).next(); | ||
| let opt_last = self.db.iterator_cf(cf_handle, IteratorMode::End).next(); | ||
| assert_eq!(opt_first.is_some(), opt_last.is_some()); |
Add delete_all to StoreUpdate and implement it as a rocksdb range delete. Use it in reset_data_pre_state_sync to clear ColState. Test plan --------- unit test existing tests
Add delete_all to StoreUpdate and implement it as a rocksdb range
delete. Use it in reset_data_pre_state_sync to clear ColState.
Test plan
unit test
existing tests