Fix test OOM issues on tree-states-hot

[michaelsproul]

WIP branch to debug test OOMs

[michaelsproul]

The Heaptrack commit should probably be ripped out and pushed to unstable separately.

[michaelsproul]

The current test failure is:

---- beacon_block_streamer::tests::check_all_blocks_from_altair_to_fulu stdout ----

thread 'beacon_block_streamer::tests::check_all_blocks_from_altair_to_fulu' panicked at beacon_node/beacon_chain/src/test_utils.rs:2712:14:
called `Result::unwrap()` on an `Err` value: BeaconChainError(DBError(HotColdDBError(HdiffGetPriorStateRootError(Slot(96), Slot(0)))))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[dapplion]

@michaelsproul there's a key design decision when using the summaries for iteration.

lighthouse/beacon_node/store/src/hot_cold_store.rs

Lines 3408 to 3414 in 2b050f4

	let mut state_root = *from_state
	.get_state_root(
	from_state
	.slot()
	.saturating_sub(Slot::new(E::SlotsPerHistoricalRoot::to_u64())),
	)
	.map_err(|_| StateSummaryIteratorError::OutOfBoundsInitialSlot)?;

What slot do we start from?

state_slot - 1

If we are attempting to store state A to the store, are we guaranteed that the summary of its parent is already persisted in disk? Could it be it will be written eventually but it's currently sitting in an ops vector? What about the migration path?

state_slot - SLOTS_PER_HISTORICAL_VECTOR

This feels safer due to the big distance, but it can experience the same issue as above. Also, we could have a situation where we are attempting to get the ancestor root of an HDiff with slot < finalized_slot. And state_slot - SLOTS_PER_HISTORICAL_VECTOR is also < finalized_slot. In that case we can get a missing state summary as it has already been pruned as part of the migration.

What I'm setting for now is:

let initial_state_slot = max(state.slot() - SLOTS_PER_HISTORICAL_VECTOR, store.split_slot);

What do you think?

[dapplion]

Test weak_subjectivity_sync_unaligned_unadvanced_checkpoint fails because the forwards block root iterator of both nodes don't match

• left = forwards: (0x6b6eef0cc6be9c55ca95d3bff66f1a78aa2aee8d2c73e3a0f958992a57999cfb, Slot(69))
• right = expected: (0xbf7bc306723de95400be0d8f4eccb9965c4b5ea5c10752d1ad84c94693c6f4a7, Slot(69))

The test weak_subjectivity_sync_unaligned_advanced_checkpoint has the exact same assertion error

@michaelsproul this tests look strange to me, questions

• weak_subjectivity_sync_unaligned_unadvanced_checkpoint uses as checkpoint a slot that's not aligned, do we support this case?
• If we checkpoint sync into slot 72, where in the test are historical blocks fetched? I see this log in the consumer node beacon_chain::historical_blocks: Ignoring some historic blocks oldest_block_slot=0 total_blocks=68 ignored=68
• Does the consumer node have backfill and range sync enabled?

EDIT: The right, expected is correct. This is the chain of blocks built

• 68 0x6b6eef0cc6be9c55ca95d3bff66f1a78aa2aee8d2c73e3a0f958992a57999cfb
• 69 0xbf7bc306723de95400be0d8f4eccb9965c4b5ea5c10752d1ad84c94693c6f4a7 <- checkpoint block
• 70 skipped
• 71 skipped
• 72 skipped
• 73 0xde74591ddcd9fe0020fccffd9e13df0e3edc408af23e76db1c52a323cc1936ca

Somehow left thinks that block 0xbf7bc306723de95400be0d8f4eccb9965c4b5ea5c10752d1ad84c94693c6f4a7 is for slot 72 in the iterator.

• The forced import historical blocks imports up to 68 included

Logs of the source checkpoint sync server node

2025-03-20T18:46:48.977158Z DEBUG store::hot_cold_store: Storing hot state summary and diffs state_root=0x305fb468655195467b0a9a763714df9cb910c5a0d3a5bbf4009928fe21c74f21 slot=68 storage_strategy=ReplayFrom(Slot(64)) diff_base_state_root=64/0
x3ce62a358919b37aff4691b697b433e882f4037d88b905bd23606cddcc38d553 previous_state_root=0xc4f4a9e0266f78efc1b46ef06a8dae69131ccd9d07c46af392d625fb92aa87c5
2025-03-20T18:46:48.977204Z DEBUG beacon_chain::beacon_chain: Beacon block imported block_root=0x6b6eef0cc6be9c55ca95d3bff66f1a78aa2aee8d2c73e3a0f958992a57999cfb block_slot=68 source=range_sync

2025-03-20T18:46:48.987348Z DEBUG store::hot_cold_store: Storing hot state summary and diffs state_root=0x4698679e674a2d6e125b0e7c3eabbd9a3434213e79acabae07de52a8946d68ea slot=69 storage_strategy=ReplayFrom(Slot(64)) diff_base_state_root=64/0
x3ce62a358919b37aff4691b697b433e882f4037d88b905bd23606cddcc38d553 previous_state_root=0x305fb468655195467b0a9a763714df9cb910c5a0d3a5bbf4009928fe21c74f21
2025-03-20T18:27:21.284430Z DEBUG beacon_chain::beacon_chain: Beacon block imported block_root=0xbf7bc306723de95400be0d8f4eccb9965c4b5ea5c10752d1ad84c94693c6f4a7 block_slot=69 source=range_sync

2025-03-20T18:46:48.997590Z DEBUG store::hot_cold_store: Storing hot state summary and diffs state_root=0x14500ce15bc46fe39594aec134e6323aee2a012dc961f67cfd95e83daa71014c slot=70 storage_strategy=ReplayFrom(Slot(64)) diff_base_state_root=64/0
x3ce62a358919b37aff4691b697b433e882f4037d88b905bd23606cddcc38d553 previous_state_root=0x4698679e674a2d6e125b0e7c3eabbd9a3434213e79acabae07de52a8946d68ea
2025-03-20T18:46:48.997656Z DEBUG store::hot_cold_store: Storing hot state summary and diffs state_root=0xfee996191ecd94ee2b0e0d0bbd22b4784982b294a8fa7ae1fd5b8c9766277ef9 slot=71 storage_strategy=ReplayFrom(Slot(64)) diff_base_state_root=64/0
x3ce62a358919b37aff4691b697b433e882f4037d88b905bd23606cddcc38d553 previous_state_root=0x14500ce15bc46fe39594aec134e6323aee2a012dc961f67cfd95e83daa71014c
2025-03-20T18:46:48.997790Z DEBUG store::hot_cold_store: Storing hot state summary and diffs state_root=0x92c13c4dce9c76f9362cc679a6d08e8967d56da6ee2f6780f76522cda2c84223 slot=72 storage_strategy=ReplayFrom(Slot(64)) diff_base_state_root=64/0
x3ce62a358919b37aff4691b697b433e882f4037d88b905bd23606cddcc38d553 previous_state_root=0xfee996191ecd94ee2b0e0d0bbd22b4784982b294a8fa7ae1fd5b8c9766277ef9
2025-03-20T18:46:48.997987Z DEBUG store::hot_cold_store: Storing hot state summary and diffs state_root=0x5669b2d3a9e8127703814aa438ccd5bb9d4a8d6e3b1e2e288cc48c9bd6d94a21 slot=73 storage_strategy=ReplayFrom(Slot(64)) diff_base_state_root=64/0
x3ce62a358919b37aff4691b697b433e882f4037d88b905bd23606cddcc38d553 previous_state_root=0x92c13c4dce9c76f9362cc679a6d08e8967d56da6ee2f6780f76522cda2c84223

2025-03-20T18:27:21.291515Z DEBUG beacon_chain::beacon_chain: Beacon block imported block_root=0xde74591ddcd9fe0020fccffd9e13df0e3edc408af23e76db1c52a323cc1936ca block_slot=73 source=range_sync

2025-03-20T18:46:49.008362Z DEBUG store::hot_cold_store: Storing hot state summary and diffs state_root=0x8ddf4321d5c3e8b887e99ac27e3da5306cadfd16becc97f6eafa1423ac457568 slot=74 storage_strategy=ReplayFrom(Slot(64)) diff_base_state_root=64/0
x3ce62a358919b37aff4691b697b433e882f4037d88b905bd23606cddcc38d553 previous_state_root=0x5669b2d3a9e8127703814aa438ccd5bb9d4a8d6e3b1e2e288cc48c9bd6d94a21
2025-03-20T18:46:49.008385Z DEBUG beacon_chain::beacon_chain: Beacon block imported block_root=0x3c6a5a9728a05e1a2fc324a41ba24e421a5ad2f57aca0794c318adf3bd4bd2ab block_slot=74 source=range_sync

Logs of the consumer node

2025-03-20T18:46:49.387830Z DEBUG beacon_chain::builder: Storing split from weak subjectivity state slot=72 state_root=0x92c13c4dce9c76f9362cc679a6d08e8967d56da6ee2f6780f76522cda2c84223 block_root=0xbf7bc306723de95400be0d8f4eccb9965c4b5ea5c10
752d1ad84c94693c6f4a7
2025-03-20T18:46:49.388062Z DEBUG store::hot_cold_store: Storing hot state summary and diffs state_root=0x92c13c4dce9c76f9362cc679a6d08e8967d56da6ee2f6780f76522cda2c84223 slot=72 storage_strategy=Snapshot diff_base_state_root=0/0x000000000000
0000000000000000000000000000000000000000000000000000 previous_state_root=0xfee996191ecd94ee2b0e0d0bbd22b4784982b294a8fa7ae1fd5b8c9766277ef9
2025-03-20T18:46:49.389877Z  INFO beacon_chain::builder: Beacon chain initialized head_state=0x4698679e674a2d6e125b0e7c3eabbd9a3434213e79acabae07de52a8946d68ea head_block=0xbf7bc306723de95400be0d8f4eccb9965c4b5ea5c10752d1ad84c94693c6f4a7 head
_slot=69

^ It has a head at slot 69 for block 0xbf7bc306723de95400be0d8f4eccb9965c4b5ea5c10752d1ad84c94693c6f4a7

[michaelsproul]

weak_subjectivity_sync_unaligned_unadvanced_checkpoint uses as checkpoint a slot that's not aligned, do we support this case?

Yeah we do. The test should pass as-is. To me this failure looks like a bug in our changes rather than an issue with the test.

If we checkpoint sync into slot 72, where in the test are historical blocks fetched?

Right here:

lighthouse/beacon_node/beacon_chain/tests/store_tests.rs

Lines 2449 to 2476 in de51d14

	// Supply blocks backwards to reach genesis. Omit the genesis block to check genesis handling.
	let historical_blocks = chain_dump[..wss_block.slot().as_usize()]
	.iter()
	.filter(|s| s.beacon_block.slot() != 0)
	.map(|s| s.beacon_block.clone())
	.collect::<Vec<_>>();
	let mut available_blocks = vec![];
	for blinded in historical_blocks {
	let block_root = blinded.canonical_root();
	let full_block = harness
	.chain
	.get_block(&block_root)
	.await
	.expect("should get block")
	.expect("should get block");
	if let MaybeAvailableBlock::Available(block) = harness
	.chain
	.data_availability_checker
	.verify_kzg_for_rpc_block(
	harness.build_rpc_block_from_store_blobs(Some(block_root), Arc::new(full_block)),
	)
	.expect("should verify kzg")
	{
	available_blocks.push(block);
	}
	}

lighthouse/beacon_node/beacon_chain/tests/store_tests.rs

Lines 2499 to 2501 in de51d14

	beacon_chain
	.import_historical_block_batch(available_blocks_dup)
	.unwrap();

Does the consumer node have backfill and range sync enabled?

Not exactly, we are manually feeding it blocks backwards and forwards to simulate backfill & range sync.

Probably just an off by one in one of the new iterators created?

[michaelsproul]

For the backwards state root iterator, I think we might need to support fetching state summaries prior to the split, right? Because we allow diffs to exist in that region (prior to the split) if they are required to construct the split state itself.

[dapplion]

Setting this const here innadvertedly mutated the anchor_slot, which would have broken HDiff reads

[dapplion]

We should rename anchor_slot and carefully review where it is mutated or set

[dapplion]

WIP, I will merge this branch first once CI is green then we can do the rename
https://github.com/sigp/lighthouse/compare/tree-states-hot-rebase-oom...dapplion:lighthouse:tree-states-hot-anchor-slot-rename?expand=1

[michaelsproul]

Not doing this for now, seeing as anchor_slot seems to be working 🤞

[dapplion]

Does this handle the genesis well?

[michaelsproul]

This could fail if state_root is pruned between when we took the split lock. We should probably hold the split lock from the start of this function (after the lookup in the state misses). This should be OK for performance because loading summaries is cheap and we're never iterating that far back.