Skip to content

Commit

Permalink
chore: remove or improve resharding TODOs (#12816)
Browse files Browse the repository at this point in the history
- Removing a few TODOs which aren't valid anymore
- Adding more context to an existing TODO
  • Loading branch information
Trisfald authored Jan 28, 2025
1 parent 8097202 commit ba1ed9f
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 11 deletions.
4 changes: 3 additions & 1 deletion chain/chain/src/resharding/resharding_actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ impl HandlerWithContext<FlatStorageShardCatchupRequest> for ReshardingActor {

impl Handler<MemtrieReloadRequest> for ReshardingActor {
fn handle(&mut self, _msg: MemtrieReloadRequest) {
// TODO(resharding)
// TODO(resharding): implement memtrie reloading. At this stage the flat storage for
// `msg.shard_uid` should be ready. Construct a new memtrie in the background and replace
// with hybrid memtrie with it. Afterwards, the hybrid memtrie can be deleted.
}
}

Expand Down
3 changes: 1 addition & 2 deletions chain/chain/src/resharding/resharding_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,7 @@ impl Chain {
let TrieUpdateBatch { entries } = batch;
let (store_update, apply_time) = {
let timer = Instant::now();
// TODO(#9435): This is highly inefficient as for each key in the batch, we are parsing the account_id
// A better way would be to use the boundary account to construct the from and to key range for flat storage iterator
// A better way would be to use the boundary account to construct the from and to key range for flat storage iterator.
let (store_update, new_state_roots) = tries.add_values_to_children_states(
&state_roots,
entries,
Expand Down
1 change: 0 additions & 1 deletion chain/chunks/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ impl ShardedTransactionPool {
/// the new shard layout.
/// It works by emptying the pools for old shard uids and re-inserting the
/// transactions back to the pool with the new shard uids.
/// TODO(resharding) check if this logic works in resharding V3
pub fn reshard(&mut self, old_shard_layout: &ShardLayout, new_shard_layout: &ShardLayout) {
tracing::debug!(
target: "resharding",
Expand Down
1 change: 0 additions & 1 deletion chain/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1800,7 +1800,6 @@ impl Client {
// layout is changing we need to reshard the transaction pool.
// TODO make sure transactions don't get added for the old shard
// layout after the pool resharding
// TODO(resharding) check if this logic works in resharding V3
if self.epoch_manager.is_next_block_epoch_start(&block_hash).unwrap_or(false) {
let new_shard_layout =
self.epoch_manager.get_shard_layout_from_prev_block(&block_hash);
Expand Down
8 changes: 2 additions & 6 deletions core/store/src/flat/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,8 @@ impl FlatStorageManager {
}
let original_value = flat_storages.insert(shard_uid, flat_storage);
if original_value.is_some() {
// Generally speaking this shouldn't happen. It may only happen when
// the node is restarted in the middle of resharding.
//
// TODO(resharding) It would be better to detect when building state
// is finished for a shard and skip doing it again after restart. We
// can then assert that the flat storage is only created once.
// Generally speaking this shouldn't happen. Starting from resharding V3 it shouldn't
// happen even if the node is restarted.
tracing::warn!(target: "store", ?shard_uid, "Creating flat storage for shard that already has flat storage.");
}
Ok(())
Expand Down

0 comments on commit ba1ed9f

Please sign in to comment.