fix(resharding): Allow creating the flat storage multiple times for a shard. #10696
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10696 +/- ##
==========================================
- Coverage 72.15% 72.14% -0.01%
==========================================
Files 737 737
Lines 150856 150858 +2
Branches 150856 150858 +2
==========================================
- Hits 108846 108841 -5
- Misses 37050 37066 +16
+ Partials 4960 4951 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
nice, gonna re run the testing with this commit, since it is slightly different from the one i ran with on friday (where i just had it not recreate the flat storage). most likely its gonna be similar? but in any case i'll try running the test again |
… shard. (near#10696) Removing the assertion and allowing flat storage to be created multiple times for a shard. This is needed so fix an issue when node is restarted in the middle of resharding. The flat storage may be created already for a subset of shards but unless all are finished resharding will get restarted. Becuase the flat storage was created, for those shards, it will be created on node startup as well as after the second resharding is finished. This is not a perfect solution and not particularly clean. The best alternative seems to be to implement resuming of resharding where we don't restart resharding for shards that were finished. This is more a comlex change and we want to get this PR in to the release so for now I'm sticking to the simplest approach. This seems to be safe because even though the flat storage for children shards is created it's not used anywhere. Sanity check - do we ever check the existance of flat storage for a shard for anything?
|
Was everything all right in the test you did with your change? I'm not aware of any real difference between the two implementations as of now, I just wanted to ensure that in case some state is preserved in the FlatStorage object we always have the most recent one - hence always insert. |
makes sense. yeah it seemed to run fine, aside from a probably unrelated stack overflow that we have seen before on testnet |
… shard. (#10696) Removing the assertion and allowing flat storage to be created multiple times for a shard. This is needed so fix an issue when node is restarted in the middle of resharding. The flat storage may be created already for a subset of shards but unless all are finished resharding will get restarted. Becuase the flat storage was created, for those shards, it will be created on node startup as well as after the second resharding is finished. This is not a perfect solution and not particularly clean. The best alternative seems to be to implement resuming of resharding where we don't restart resharding for shards that were finished. This is more a comlex change and we want to get this PR in to the release so for now I'm sticking to the simplest approach. This seems to be safe because even though the flat storage for children shards is created it's not used anywhere. Sanity check - do we ever check the existance of flat storage for a shard for anything?
Removing the assertion and allowing flat storage to be created multiple times for a shard. This is needed so fix an issue when node is restarted in the middle of resharding. The flat storage may be created already for a subset of shards but unless all are finished resharding will get restarted. Becuase the flat storage was created, for those shards, it will be created on node startup as well as after the second resharding is finished.
This is not a perfect solution and not particularly clean. The best alternative seems to be to implement resuming of resharding where we don't restart resharding for shards that were finished. This is more a comlex change and we want to get this PR in to the release so for now I'm sticking to the simplest approach.
This seems to be safe because even though the flat storage for children shards is created it's not used anywhere.
Sanity check - do we ever check the existance of flat storage for a shard for anything?