From e4c1b5991c2c060080a1c9e349170e889070b792 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Wed, 9 Oct 2024 20:01:54 +1000 Subject: [PATCH] Fix RocksDBException: Busy during snapsync (#7746) Signed-off-by: Karim Taam Signed-off-by: Simon Dudley Co-authored-by: Karim Taam --- CHANGELOG.md | 3 +- .../request/heal/TrieNodeHealingRequest.java | 9 ++-- .../sync/snapsync/PersistDataStepTest.java | 48 ++++++++++++++++++- 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 592b445bc8..a9e5beb331 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,8 @@ - Corrects a regression where custom plugin services are not initialized correctly. [#7625](https://github.com/hyperledger/besu/pull/7625) - Fix for IBFT2 chains using the BONSAI DB format [#7631](https://github.com/hyperledger/besu/pull/7631) - Fix reading `tx-pool-min-score` option from configuration file [#7623](https://github.com/hyperledger/besu/pull/7623) -- Fix an undhandled exception. [#7733](https://github.com/hyperledger/besu/issues/7733) +- Fix an unhandled PeerTable exception [#7733](https://github.com/hyperledger/besu/issues/7733) +- Fix RocksDBException: Busy leading to MerkleTrieException: Unable to load trie node value [#7745](https://github.com/hyperledger/besu/pull/7745) ## 24.9.1 diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/TrieNodeHealingRequest.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/TrieNodeHealingRequest.java index 7e206232c5..e622108725 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/TrieNodeHealingRequest.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/request/heal/TrieNodeHealingRequest.java @@ -32,6 +32,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Stream; import org.apache.tuweni.bytes.Bytes; @@ -44,7 +45,7 @@ public abstract class TrieNodeHealingRequest extends SnapDataRequest private final Bytes location; protected Bytes data; - protected boolean requiresPersisting = true; + protected AtomicBoolean requiresPersisting = new AtomicBoolean(true); protected TrieNodeHealingRequest(final Hash nodeHash, final Hash rootHash, final Bytes location) { super(TRIE_NODE, rootHash); @@ -65,7 +66,7 @@ public abstract class TrieNodeHealingRequest extends SnapDataRequest return 0; } int saved = 0; - if (requiresPersisting) { + if (requiresPersisting.getAndSet(false)) { checkNotNull(data, "Must set data before node can be persisted."); saved = doPersist( @@ -143,7 +144,7 @@ public abstract class TrieNodeHealingRequest extends SnapDataRequest } public boolean isRequiresPersisting() { - return requiresPersisting; + return requiresPersisting.get(); } public Bytes32 getNodeHash() { @@ -173,7 +174,7 @@ public abstract class TrieNodeHealingRequest extends SnapDataRequest } public void setRequiresPersisting(final boolean requiresPersisting) { - this.requiresPersisting = requiresPersisting; + this.requiresPersisting.set(requiresPersisting); } private boolean nodeIsHashReferencedDescendant(final Node node) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java index 0364dc7581..766995b094 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/snapsync/PersistDataStepTest.java @@ -17,6 +17,9 @@ package org.hyperledger.besu.ethereum.eth.sync.snapsync; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.hyperledger.besu.datatypes.Hash; @@ -25,12 +28,15 @@ import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.AccountRangeDataR import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.BytecodeRequest; import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.SnapDataRequest; import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.StorageRangeDataRequest; +import org.hyperledger.besu.ethereum.eth.sync.snapsync.request.heal.AccountTrieNodeHealingRequest; import org.hyperledger.besu.ethereum.trie.diffbased.bonsai.storage.BonsaiWorldStateKeyValueStorage; import org.hyperledger.besu.ethereum.trie.patricia.StoredMerklePatriciaTrie; import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration; +import org.hyperledger.besu.ethereum.worldstate.WorldStateKeyValueStorage; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorageCoordinator; import org.hyperledger.besu.services.tasks.Task; +import java.util.Collections; import java.util.List; import org.apache.tuweni.bytes.Bytes; @@ -39,9 +45,12 @@ import org.junit.jupiter.api.Test; public class PersistDataStepTest { + private final WorldStateKeyValueStorage worldStateKeyValueStorage = + spy( + new InMemoryKeyValueStorageProvider() + .createWorldStateStorage(DataStorageConfiguration.DEFAULT_CONFIG)); private final WorldStateStorageCoordinator worldStateStorageCoordinator = - new InMemoryKeyValueStorageProvider() - .createWorldStateStorageCoordinator(DataStorageConfiguration.DEFAULT_CONFIG); + new WorldStateStorageCoordinator(worldStateKeyValueStorage); private final SnapSyncProcessState snapSyncState = mock(SnapSyncProcessState.class); private final SnapWorldDownloadState downloadState = mock(SnapWorldDownloadState.class); @@ -67,6 +76,33 @@ public class PersistDataStepTest { assertDataPersisted(tasks); } + @Test + public void shouldPersistTrieNodeHealDataOnlyOnce() { + + final Bytes stateTrieNode = + Bytes.fromHexString( + "0xe2a0310e2d527612073b26eecdfd717e6a320cf44b4afac2b0732d9fcbe2b7fa0cf602"); + final Hash hash = Hash.hash(stateTrieNode); + final Bytes location = Bytes.of(0x02); + final AccountTrieNodeHealingRequest accountTrieNodeDataRequest = + SnapDataRequest.createAccountTrieNodeDataRequest(hash, location, Collections.emptySet()); + accountTrieNodeDataRequest.setData(stateTrieNode); + + final BonsaiWorldStateKeyValueStorage.Updater updater = + (BonsaiWorldStateKeyValueStorage.Updater) spy(worldStateKeyValueStorage.updater()); + when(worldStateKeyValueStorage.updater()) + .thenReturn(updater) + .thenReturn(mock(BonsaiWorldStateKeyValueStorage.Updater.class)); + + List> result = + persistDataStep.persist(List.of(new StubTask(accountTrieNodeDataRequest))); + + persistDataStep.persist(List.of(new StubTask(accountTrieNodeDataRequest))); + + verify(updater, times(1)).putAccountStateTrieNode(location, hash, stateTrieNode); + assertDataPersisted(result); + } + @Test public void shouldSkipPersistDataWhenNoData() { final List> tasks = TaskGenerator.createAccountRequest(false); @@ -110,6 +146,14 @@ public class PersistDataStepTest { .getStrategy(BonsaiWorldStateKeyValueStorage.class) .getCode(Hash.wrap(data.getCodeHash()), Hash.wrap(data.getAccountHash()))) .isPresent(); + } else if (task.getData() instanceof AccountTrieNodeHealingRequest) { + final AccountTrieNodeHealingRequest data = + (AccountTrieNodeHealingRequest) task.getData(); + assertThat( + worldStateStorageCoordinator + .getStrategy(BonsaiWorldStateKeyValueStorage.class) + .getTrieNodeUnsafe(data.getLocation())) + .isPresent(); } else { fail("not expected message"); }