Skip to content

Commit

Permalink
fix(test): gc_sync_after_sync (#3160)
Browse files Browse the repository at this point in the history
gc_sync_after_sync_* tests fail for the following reasons:
1) fork_tail is not properly maintained. A node resets fork tail after state sync but we allowed it to go back on epoch change.
2) in #3099 I didn't account for the fact that we can have chunk in storage whose height is higher than the current head.

Fixes #3133.

Test plan
----------
Run each of the gc_sync_after_sync_* tests 30 tests and observe that there is 0 failure.
  • Loading branch information
bowenwang1996 authored Aug 13, 2020
1 parent 82501ae commit c9d37cf
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions chain/chain/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,17 +529,16 @@ impl Chain {
}
let prev_epoch_id = self.get_block_header(&head.prev_block_hash)?.epoch_id();
let epoch_change = prev_epoch_id != &head.epoch_id;
let fork_tail = if epoch_change {
let mut fork_tail = self.store.fork_tail()?;
if epoch_change && fork_tail < gc_stop_height {
// if head doesn't change on the epoch boundary, we may update fork tail several times
// but that is fine since it doesn't affect correctness and also we limit the number of
// heights that fork cleaning goes through so it doesn't slow down client either.
let mut chain_store_update = self.store.store_update();
chain_store_update.update_fork_tail(gc_stop_height);
chain_store_update.commit()?;
gc_stop_height
} else {
self.store.fork_tail()?
};
fork_tail = gc_stop_height;
}
let mut gc_blocks_remaining = gc_blocks_limit;

// Forks Cleaning
Expand Down Expand Up @@ -925,7 +924,8 @@ impl Chain {
// Get header we were syncing into.
let header = self.get_block_header(&sync_hash)?;
let prev_hash = *header.prev_hash();
let gc_height = std::cmp::min(head.height + 1, header.height());
let sync_height = header.height();
let gc_height = std::cmp::min(head.height + 1, sync_height);

// GC all the data from current tail up to `gc_height`
let tail = self.store.tail()?;
Expand All @@ -946,7 +946,9 @@ impl Chain {

// Clear Chunks data
let mut chain_store_update = self.mut_store().store_update();
chain_store_update.clear_chunk_data(gc_height)?;
// The largest height of chunk we have in storage is head.height + 1
let chunk_height = std::cmp::min(head.height + 2, sync_height);
chain_store_update.clear_chunk_data(chunk_height)?;
chain_store_update.commit()?;

// clear all trie data
Expand Down Expand Up @@ -996,6 +998,7 @@ impl Chain {
// New Tail can not be earlier than `prev_block.header.inner_lite.height`
chain_store_update.update_tail(new_tail);
// New Chunk Tail can not be earlier than minimum of height_created in Block `prev_block`
println!("resetting chunk tail to {}", new_chunk_tail);
chain_store_update.update_chunk_tail(new_chunk_tail);
chain_store_update.commit()?;

Expand Down

0 comments on commit c9d37cf

Please sign in to comment.