From 2a89edd5b173ad84311a24937a75301f4f7befce Mon Sep 17 00:00:00 2001 From: garyschulte Date: Thu, 22 Dec 2022 14:20:54 -0800 Subject: [PATCH] Bugfix snapshot transaction segfaults after storage truncation (#4786) * subscribe snapshot worldstates to parent worldstate storage events like clear and clearFlatDatabase and close as appropriate to avoid segfaults * fix for direct snapshot creation when using snapshot archive * ensure we only prune bonsai worldstates if we have more than the configured number of retained states Signed-off-by: garyschulte --- .../blockcreation/AbstractBlockCreator.java | 12 +- .../bonsai/AbstractTrieLogManager.java | 97 +++++++------ .../bonsai/BonsaiInMemoryWorldState.java | 9 +- .../bonsai/BonsaiLayeredWorldState.java | 21 +-- .../bonsai/BonsaiPersistedWorldState.java | 15 +- .../bonsai/BonsaiSnapshotWorldState.java | 55 +++++--- ...nsaiSnapshotWorldStateKeyValueStorage.java | 87 ++++++++++-- .../bonsai/BonsaiWorldStateArchive.java | 8 +- .../BonsaiWorldStateKeyValueStorage.java | 30 ++-- .../bonsai/CachedMerkleTrieLoader.java | 11 +- .../bonsai/LayeredTrieLogManager.java | 26 ++-- .../bonsai/SnapshotTrieLogManager.java | 95 ++++++++----- .../besu/ethereum/bonsai/TrieLogManager.java | 17 +-- .../core/SnapshotMutableWorldState.java | 6 +- .../bonsai/AbstractIsolationTests.java | 8 +- .../bonsai/BonsaiSnapshotIsolationTests.java | 15 ++ .../BonsaiSnapshotWorldStateArchiveTest.java | 24 ++-- .../bonsai/BonsaiWorldStateArchiveTest.java | 7 +- .../bonsai/LayeredWorldStateTests.java | 131 ++++++++++++++++++ .../rocksdb/segmented/RocksDBSnapshot.java | 4 +- .../segmented/RocksDBSnapshotTransaction.java | 20 +++ 21 files changed, 505 insertions(+), 193 deletions(-) create mode 100644 ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/LayeredWorldStateTests.java diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java index aa6b27d95c51..a973ec2c0f16 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java @@ -256,16 +256,8 @@ private MutableWorldState duplicateWorldStateAtParent() { if (ws.isPersistable()) { return ws; } else { - var wsCopy = ws.copy(); - try { - ws.close(); - } catch (Exception ex) { - LOG.error( - "unexpected error closing non-peristable worldstate + " - + parentHeader.toLogString(), - ex); - } - return wsCopy; + // non-persistable worldstates should return a copy which is persistable: + return ws.copy(); } }) .orElseThrow( diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/AbstractTrieLogManager.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/AbstractTrieLogManager.java index 0296cf18eff0..7ac086bbaa1d 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/AbstractTrieLogManager.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/AbstractTrieLogManager.java @@ -18,7 +18,7 @@ import static org.hyperledger.besu.util.Slf4jLambdaHelper.debugLambda; import org.hyperledger.besu.datatypes.Hash; -import org.hyperledger.besu.ethereum.bonsai.TrieLogManager.CachedWorldState; +import org.hyperledger.besu.ethereum.bonsai.BonsaiWorldStateKeyValueStorage.BonsaiUpdater; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.MutableWorldState; @@ -28,27 +28,29 @@ import java.util.Optional; import java.util.stream.Collectors; +import com.google.common.annotations.VisibleForTesting; import org.apache.tuweni.bytes.Bytes32; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public abstract class AbstractTrieLogManager implements TrieLogManager { +public abstract class AbstractTrieLogManager + implements TrieLogManager { private static final Logger LOG = LoggerFactory.getLogger(AbstractTrieLogManager.class); public static final long RETAINED_LAYERS = 512; // at least 256 + typical rollbacks protected final Blockchain blockchain; - protected final BonsaiWorldStateKeyValueStorage worldStateStorage; + protected final BonsaiWorldStateKeyValueStorage rootWorldStateStorage; - protected final Map cachedWorldStatesByHash; + protected final Map> cachedWorldStatesByHash; protected final long maxLayersToLoad; - public AbstractTrieLogManager( + AbstractTrieLogManager( final Blockchain blockchain, final BonsaiWorldStateKeyValueStorage worldStateStorage, final long maxLayersToLoad, - final Map cachedWorldStatesByHash) { + final Map> cachedWorldStatesByHash) { this.blockchain = blockchain; - this.worldStateStorage = worldStateStorage; + this.rootWorldStateStorage = worldStateStorage; this.cachedWorldStatesByHash = cachedWorldStatesByHash; this.maxLayersToLoad = maxLayersToLoad; } @@ -57,19 +59,24 @@ public AbstractTrieLogManager( public synchronized void saveTrieLog( final BonsaiWorldStateArchive worldStateArchive, final BonsaiWorldStateUpdater localUpdater, - final Hash worldStateRootHash, - final BlockHeader blockHeader) { + final Hash forWorldStateRootHash, + final BlockHeader forBlockHeader, + final BonsaiPersistedWorldState forWorldState) { // do not overwrite a trielog layer that already exists in the database. // if it's only in memory we need to save it // for example, in case of reorg we don't replace a trielog layer - if (worldStateStorage.getTrieLog(blockHeader.getHash()).isEmpty()) { - final BonsaiWorldStateKeyValueStorage.BonsaiUpdater stateUpdater = - worldStateStorage.updater(); + if (rootWorldStateStorage.getTrieLog(forBlockHeader.getHash()).isEmpty()) { + final BonsaiUpdater stateUpdater = forWorldState.getWorldStateStorage().updater(); boolean success = false; try { final TrieLogLayer trieLog = - prepareTrieLog(blockHeader, worldStateRootHash, localUpdater, worldStateArchive); - persistTrieLog(blockHeader, worldStateRootHash, trieLog, stateUpdater); + prepareTrieLog( + forBlockHeader, + forWorldStateRootHash, + localUpdater, + worldStateArchive, + forWorldState); + persistTrieLog(forBlockHeader, forWorldStateRootHash, trieLog, stateUpdater); success = true; } finally { if (success) { @@ -81,45 +88,57 @@ public synchronized void saveTrieLog( } } - private TrieLogLayer prepareTrieLog( + protected abstract void addCachedLayer( final BlockHeader blockHeader, - final Hash currentWorldStateRootHash, + final Hash worldStateRootHash, + final TrieLogLayer trieLog, + final BonsaiWorldStateArchive worldStateArchive, + final BonsaiPersistedWorldState forWorldState); + + @VisibleForTesting + TrieLogLayer prepareTrieLog( + final BlockHeader blockHeader, + final Hash worldStateRootHash, final BonsaiWorldStateUpdater localUpdater, - final BonsaiWorldStateArchive worldStateArchive) { + final BonsaiWorldStateArchive worldStateArchive, + final BonsaiPersistedWorldState forWorldState) { debugLambda(LOG, "Adding layered world state for {}", blockHeader::toLogString); final TrieLogLayer trieLog = localUpdater.generateTrieLog(blockHeader.getBlockHash()); trieLog.freeze(); - addCachedLayer(blockHeader, currentWorldStateRootHash, trieLog, worldStateArchive); + addCachedLayer(blockHeader, worldStateRootHash, trieLog, worldStateArchive, forWorldState); scrubCachedLayers(blockHeader.getNumber()); return trieLog; } synchronized void scrubCachedLayers(final long newMaxHeight) { - final long waterline = newMaxHeight - RETAINED_LAYERS; - cachedWorldStatesByHash.values().stream() - .filter(layer -> layer.getHeight() < waterline) - .collect(Collectors.toList()) - .stream() - .forEach( - layer -> { - cachedWorldStatesByHash.remove(layer.getTrieLog().getBlockHash()); - Optional.ofNullable(layer.getMutableWorldState()) - .ifPresent( - ws -> { - try { - ws.close(); - } catch (Exception e) { - LOG.warn("Error closing bonsai worldstate layer", e); - } - }); - }); + if (cachedWorldStatesByHash.size() > RETAINED_LAYERS) { + final long waterline = newMaxHeight - RETAINED_LAYERS; + cachedWorldStatesByHash.values().stream() + .filter(layer -> layer.getHeight() < waterline) + .collect(Collectors.toList()) + .stream() + .forEach( + layer -> { + cachedWorldStatesByHash.remove(layer.getTrieLog().getBlockHash()); + layer.dispose(); + Optional.ofNullable(layer.getMutableWorldState()) + .ifPresent( + ws -> { + try { + ws.close(); + } catch (Exception e) { + LOG.warn("Error closing bonsai worldstate layer", e); + } + }); + }); + } } private void persistTrieLog( final BlockHeader blockHeader, final Hash worldStateRootHash, final TrieLogLayer trieLog, - final BonsaiWorldStateKeyValueStorage.BonsaiUpdater stateUpdater) { + final BonsaiUpdater stateUpdater) { debugLambda( LOG, "Persisting trie log for block hash {} and world state root {}", @@ -136,7 +155,7 @@ private void persistTrieLog( public Optional getBonsaiCachedWorldState(final Hash blockHash) { if (cachedWorldStatesByHash.containsKey(blockHash)) { return Optional.ofNullable(cachedWorldStatesByHash.get(blockHash)) - .map(T::getMutableWorldState); + .map(CachedWorldState::getMutableWorldState); } return Optional.empty(); } @@ -151,7 +170,7 @@ public Optional getTrieLogLayer(final Hash blockHash) { if (cachedWorldStatesByHash.containsKey(blockHash)) { return Optional.of(cachedWorldStatesByHash.get(blockHash).getTrieLog()); } else { - return worldStateStorage.getTrieLog(blockHash).map(TrieLogLayer::fromBytes); + return rootWorldStateStorage.getTrieLog(blockHash).map(TrieLogLayer::fromBytes); } } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiInMemoryWorldState.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiInMemoryWorldState.java index 7eb09a0bba0a..64ee2c42fbe3 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiInMemoryWorldState.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiInMemoryWorldState.java @@ -18,6 +18,7 @@ import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.bonsai.BonsaiWorldStateKeyValueStorage.BonsaiStorageSubscriber; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.trie.StoredMerklePatriciaTrie; @@ -27,14 +28,17 @@ import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.units.bigints.UInt256; -public class BonsaiInMemoryWorldState extends BonsaiPersistedWorldState { +public class BonsaiInMemoryWorldState extends BonsaiPersistedWorldState + implements BonsaiStorageSubscriber { private boolean isPersisted = false; + private final Long worldstateSubcriberId; public BonsaiInMemoryWorldState( final BonsaiWorldStateArchive archive, final BonsaiWorldStateKeyValueStorage worldStateStorage) { super(archive, worldStateStorage); + worldstateSubcriberId = worldStateStorage.subscribe(this); } @Override @@ -146,7 +150,7 @@ public void persist(final BlockHeader blockHeader) { final Hash newWorldStateRootHash = rootHash(localUpdater); archive .getTrieLogManager() - .saveTrieLog(archive, localUpdater, newWorldStateRootHash, blockHeader); + .saveTrieLog(archive, localUpdater, newWorldStateRootHash, blockHeader, this); worldStateRootHash = newWorldStateRootHash; worldStateBlockHash = blockHeader.getBlockHash(); isPersisted = true; @@ -155,6 +159,7 @@ public void persist(final BlockHeader blockHeader) { @Override public void close() throws Exception { // if storage is snapshot-based we need to close: + worldStateStorage.unSubscribe(worldstateSubcriberId); worldStateStorage.close(); } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiLayeredWorldState.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiLayeredWorldState.java index bff30add2cde..222aef38ee6e 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiLayeredWorldState.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiLayeredWorldState.java @@ -21,6 +21,7 @@ import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.MutableWorldState; +import org.hyperledger.besu.ethereum.core.SnapshotMutableWorldState; import org.hyperledger.besu.ethereum.worldstate.StateTrieAccountValue; import org.hyperledger.besu.evm.account.Account; import org.hyperledger.besu.evm.worldstate.WorldState; @@ -261,14 +262,18 @@ public Stream streamAccounts(final Bytes32 startKeyHash, fina @MustBeClosed public MutableWorldState copy() { // return an in-memory worldstate that is based on a persisted snapshot for this blockhash. - return archive - .getMutableSnapshot(this.blockHash()) - .map(BonsaiSnapshotWorldState.class::cast) - .map(snapshot -> new BonsaiInMemoryWorldState(archive, snapshot.getWorldStateStorage())) - .orElseThrow( - () -> - new StorageException( - "Unable to copy Layered Worldstate for " + blockHash().toHexString())); + try (SnapshotMutableWorldState snapshot = + archive + .getMutableSnapshot(this.blockHash()) + .map(SnapshotMutableWorldState.class::cast) + .orElseThrow( + () -> + new StorageException( + "Unable to copy Layered Worldstate for " + blockHash().toHexString()))) { + return new BonsaiInMemoryWorldState(archive, snapshot.getWorldStateStorage()); + } catch (Exception ex) { + throw new RuntimeException(ex); + } } @Override diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiPersistedWorldState.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiPersistedWorldState.java index 9ffcd0183177..e093623b7c13 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiPersistedWorldState.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiPersistedWorldState.java @@ -72,11 +72,11 @@ public BonsaiPersistedWorldState( (addr, value) -> archive .getCachedMerkleTrieLoader() - .preLoadAccount(worldStateStorage, worldStateRootHash, addr), + .preLoadAccount(getWorldStateStorage(), worldStateRootHash, addr), (addr, value) -> archive .getCachedMerkleTrieLoader() - .preLoadStorageSlot(worldStateStorage, addr, value)); + .preLoadStorageSlot(getWorldStateStorage(), addr, value)); } public BonsaiWorldStateArchive getArchive() { @@ -282,6 +282,7 @@ public void persist(final BlockHeader blockHeader) { final BonsaiWorldStateUpdater localUpdater = updater.copy(); final BonsaiWorldStateKeyValueStorage.BonsaiUpdater stateUpdater = worldStateStorage.updater(); + Runnable saveTrieLog = () -> {}; try { final Hash newWorldStateRootHash = calculateRootHash(stateUpdater, localUpdater); @@ -296,9 +297,12 @@ public void persist(final BlockHeader blockHeader) { + " calculated " + newWorldStateRootHash.toHexString()); } - archive - .getTrieLogManager() - .saveTrieLog(archive, localUpdater, newWorldStateRootHash, blockHeader); + saveTrieLog = + () -> + archive + .getTrieLogManager() + .saveTrieLog(archive, localUpdater, newWorldStateRootHash, blockHeader, this); + stateUpdater .getTrieBranchStorageTransaction() .put(WORLD_BLOCK_HASH_KEY, blockHeader.getHash().toArrayUnsafe()); @@ -317,6 +321,7 @@ public void persist(final BlockHeader blockHeader) { if (success) { stateUpdater.commit(); updater.reset(); + saveTrieLog.run(); } else { stateUpdater.rollback(); updater.reset(); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldState.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldState.java index f91a922ddd83..0353b59c2fe2 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldState.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldState.java @@ -33,28 +33,36 @@ public class BonsaiSnapshotWorldState extends BonsaiPersistedWorldState private final SnappedKeyValueStorage codeSnap; private final SnappedKeyValueStorage storageSnap; private final SnappedKeyValueStorage trieBranchSnap; + private final BonsaiWorldStateKeyValueStorage parentWorldStateStorage; + private final BonsaiSnapshotWorldStateKeyValueStorage snapshotWorldStateStorage; private BonsaiSnapshotWorldState( final BonsaiWorldStateArchive archive, - final BonsaiSnapshotWorldStateKeyValueStorage snapshotWorldStateStorage) { + final BonsaiSnapshotWorldStateKeyValueStorage snapshotWorldStateStorage, + final BonsaiWorldStateKeyValueStorage parentWorldStateStorage) { super(archive, snapshotWorldStateStorage); + this.snapshotWorldStateStorage = snapshotWorldStateStorage; this.accountSnap = (SnappedKeyValueStorage) snapshotWorldStateStorage.accountStorage; this.codeSnap = (SnappedKeyValueStorage) snapshotWorldStateStorage.codeStorage; this.storageSnap = (SnappedKeyValueStorage) snapshotWorldStateStorage.storageStorage; this.trieBranchSnap = (SnappedKeyValueStorage) snapshotWorldStateStorage.trieBranchStorage; + this.parentWorldStateStorage = parentWorldStateStorage; } public static BonsaiSnapshotWorldState create( final BonsaiWorldStateArchive archive, final BonsaiWorldStateKeyValueStorage parentWorldStateStorage) { return new BonsaiSnapshotWorldState( - archive, - new BonsaiSnapshotWorldStateKeyValueStorage( - ((SnappableKeyValueStorage) parentWorldStateStorage.accountStorage).takeSnapshot(), - ((SnappableKeyValueStorage) parentWorldStateStorage.codeStorage).takeSnapshot(), - ((SnappableKeyValueStorage) parentWorldStateStorage.storageStorage).takeSnapshot(), - ((SnappableKeyValueStorage) parentWorldStateStorage.trieBranchStorage).takeSnapshot(), - parentWorldStateStorage.trieLogStorage)); + archive, + new BonsaiSnapshotWorldStateKeyValueStorage( + ((SnappableKeyValueStorage) parentWorldStateStorage.accountStorage).takeSnapshot(), + ((SnappableKeyValueStorage) parentWorldStateStorage.codeStorage).takeSnapshot(), + ((SnappableKeyValueStorage) parentWorldStateStorage.storageStorage).takeSnapshot(), + ((SnappableKeyValueStorage) parentWorldStateStorage.trieBranchStorage) + .takeSnapshot(), + parentWorldStateStorage.trieLogStorage), + parentWorldStateStorage) + .subscribeToParentStorage(); } @Override @@ -69,20 +77,29 @@ public Hash rootHash() { public MutableWorldState copy() { // return a clone-based copy of worldstate storage return new BonsaiSnapshotWorldState( - archive, - new BonsaiSnapshotWorldStateKeyValueStorage( - accountSnap.cloneFromSnapshot(), - codeSnap.cloneFromSnapshot(), - storageSnap.cloneFromSnapshot(), - trieBranchSnap.cloneFromSnapshot(), - worldStateStorage.trieLogStorage)); + archive, + new BonsaiSnapshotWorldStateKeyValueStorage( + accountSnap.cloneFromSnapshot(), + codeSnap.cloneFromSnapshot(), + storageSnap.cloneFromSnapshot(), + trieBranchSnap.cloneFromSnapshot(), + worldStateStorage.trieLogStorage), + parentWorldStateStorage) + .subscribeToParentStorage(); } @Override public void close() throws Exception { - accountSnap.close(); - codeSnap.close(); - storageSnap.close(); - trieBranchSnap.close(); + snapshotWorldStateStorage.close(); + } + + @Override + public BonsaiWorldStateKeyValueStorage getWorldStateStorage() { + return snapshotWorldStateStorage; + } + + protected BonsaiSnapshotWorldState subscribeToParentStorage() { + snapshotWorldStateStorage.subscribeToParentStorage(parentWorldStateStorage); + return this; } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldStateKeyValueStorage.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldStateKeyValueStorage.java index 3e85e368b23b..80fd4e0efe99 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldStateKeyValueStorage.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldStateKeyValueStorage.java @@ -18,29 +18,37 @@ import static org.hyperledger.besu.util.Slf4jLambdaHelper.warnLambda; import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.bonsai.BonsaiWorldStateKeyValueStorage.BonsaiStorageSubscriber; import org.hyperledger.besu.ethereum.storage.StorageProvider; import org.hyperledger.besu.ethereum.storage.keyvalue.KeyValueSegmentIdentifier; import org.hyperledger.besu.ethereum.trie.MerklePatriciaTrie; import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage; +import org.hyperledger.besu.plugin.services.exception.StorageException; import org.hyperledger.besu.plugin.services.storage.KeyValueStorage; import org.hyperledger.besu.plugin.services.storage.KeyValueStorageTransaction; import org.hyperledger.besu.plugin.services.storage.SnappedKeyValueStorage; -import org.hyperledger.besu.util.Subscribers; +import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class BonsaiSnapshotWorldStateKeyValueStorage extends BonsaiWorldStateKeyValueStorage { +public class BonsaiSnapshotWorldStateKeyValueStorage extends BonsaiWorldStateKeyValueStorage + implements BonsaiStorageSubscriber { private static final Logger LOG = LoggerFactory.getLogger(BonsaiSnapshotWorldStateKeyValueStorage.class); + private final AtomicReference parentStorage = + new AtomicReference<>(); + private final AtomicLong parentStorageSubscriberId = new AtomicLong(Long.MAX_VALUE); + private final AtomicBoolean shouldClose = new AtomicBoolean(false); private final AtomicBoolean isClosed = new AtomicBoolean(false); - private final Subscribers subscribers = Subscribers.create(); public BonsaiSnapshotWorldStateKeyValueStorage(final StorageProvider snappableStorageProvider) { this( @@ -81,16 +89,28 @@ public BonsaiUpdater updater() { } @Override - public synchronized long subscribe() { - if (isClosed.get()) { - throw new RuntimeException("BonsaiSnapshotWorldStateKeyValueStorage already closed"); + public void clear() { + // snapshot storage does not implement clear + throw new StorageException("Snapshot storage does not implement clear"); + } + + @Override + public void clearFlatDatabase() { + // snapshot storage does not implement clear + throw new StorageException("Snapshot storage does not implement clear"); + } + + @Override + public synchronized long subscribe(final BonsaiStorageSubscriber sub) { + if (shouldClose.get()) { + throw new RuntimeException("Storage is marked to close or has already closed"); } - return subscribers.subscribe(0); + return super.subscribe(sub); } @Override public synchronized void unSubscribe(final long id) { - subscribers.unsubscribe(id); + super.unSubscribe(id); try { tryClose(); } catch (Exception e) { @@ -98,18 +118,63 @@ public synchronized void unSubscribe(final long id) { } } + void subscribeToParentStorage(final BonsaiWorldStateKeyValueStorage parentStorage) { + this.parentStorage.set(parentStorage); + parentStorageSubscriberId.set(parentStorage.subscribe(this)); + } + + @Override + public void onClearStorage() { + try { + // when the parent storage clears, close regardless of subscribers + doClose(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + @Override + public void onClearFlatDatabaseStorage() { + // when the parent storage clears, close regardless of subscribers + try { + doClose(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + @Override public synchronized void close() throws Exception { - isClosed.getAndSet(true); + // when the parent storage clears, close + shouldClose.set(true); tryClose(); } - protected void tryClose() throws Exception { - if (isClosed.get() && subscribers.getSubscriberCount() < 1) { + protected synchronized void tryClose() throws Exception { + if (shouldClose.get() && subscribers.getSubscriberCount() < 1) { + // attempting to close already closed snapshots will segfault + doClose(); + } + } + + private void doClose() throws Exception { + if (!isClosed.get()) { + // alert any subscribers we are closing: + subscribers.forEach(BonsaiStorageSubscriber::onCloseStorage); + + // unsubscribe from parent storage if we have subscribed + Optional.ofNullable(parentStorage.get()) + .filter(__ -> parentStorageSubscriberId.get() != Long.MAX_VALUE) + .ifPresent(parent -> parent.unSubscribe(parentStorageSubscriberId.get())); + + // close all of the SnappedKeyValueStorages: accountStorage.close(); codeStorage.close(); storageStorage.close(); trieBranchStorage.close(); + + // set storage closed + isClosed.set(true); } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchive.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchive.java index 50bac540bb93..6126203b97ae 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchive.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchive.java @@ -115,7 +115,9 @@ public BonsaiWorldStateArchive( this.blockchain = blockchain; this.worldStateStorage = worldStateStorage; this.persistedState = new BonsaiPersistedWorldState(this, worldStateStorage); - this.useSnapshots = useSnapshots; + // TODO: https://github.com/hyperledger/besu/issues/4641 + // useSnapshots is disabled for now + this.useSnapshots = false; this.cachedMerkleTrieLoader = cachedMerkleTrieLoader; blockchain.observeBlockAdded(this::blockAddedHandler); } @@ -276,11 +278,11 @@ Optional rollMutableStateToBlockHash( } catch (final Exception e) { // if we fail we must clean up the updater bonsaiUpdater.reset(); - LOG.debug("Archive rolling failed for block hash " + blockHash, e); + LOG.debug("State rolling failed for block hash " + blockHash, e); return Optional.empty(); } } catch (final RuntimeException re) { - LOG.debug("Archive rolling failed for block hash " + blockHash, re); + LOG.error("Archive rolling failed for block hash " + blockHash, re); return Optional.empty(); } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateKeyValueStorage.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateKeyValueStorage.java index b379f4d59c2f..3d0972e9a1e2 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateKeyValueStorage.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateKeyValueStorage.java @@ -27,10 +27,10 @@ import org.hyperledger.besu.ethereum.worldstate.WorldStateStorage; import org.hyperledger.besu.plugin.services.storage.KeyValueStorage; import org.hyperledger.besu.plugin.services.storage.KeyValueStorageTransaction; +import org.hyperledger.besu.util.Subscribers; import java.nio.charset.StandardCharsets; import java.util.Optional; -import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; @@ -49,6 +49,7 @@ public class BonsaiWorldStateKeyValueStorage implements WorldStateStorage, AutoC protected final KeyValueStorage storageStorage; protected final KeyValueStorage trieBranchStorage; protected final KeyValueStorage trieLogStorage; + protected final Subscribers subscribers = Subscribers.create(); private Optional maybeFallbackNodeFinder; @@ -221,6 +222,7 @@ public boolean isWorldStateAvailable(final Bytes32 rootHash, final Hash blockHas @Override public void clear() { + subscribers.forEach(BonsaiStorageSubscriber::onClearStorage); accountStorage.clear(); codeStorage.clear(); storageStorage.clear(); @@ -230,6 +232,7 @@ public void clear() { @Override public void clearFlatDatabase() { + subscribers.forEach(BonsaiStorageSubscriber::onClearFlatDatabaseStorage); accountStorage.clear(); storageStorage.clear(); } @@ -268,24 +271,17 @@ public void useFallbackNodeFinder(final Optional maybeFallba this.maybeFallbackNodeFinder = maybeFallbackNodeFinder; } - public void safeExecute(final Consumer toExec) throws Exception { - final long id = subscribe(); - toExec.accept((KeyValueStorage) this); - unSubscribe(id); + public synchronized long subscribe(final BonsaiStorageSubscriber sub) { + return subscribers.subscribe(sub); } - public long subscribe() { - // No op because close() is not implemented for BonsaiWorldStateKeyValueStorage - return 0; - } - - public void unSubscribe(final long id) { - // No op because close() is not implemented for BonsaiWorldStateKeyValueStorage + public synchronized void unSubscribe(final long id) { + subscribers.unsubscribe(id); } @Override public void close() throws Exception { - // No need to close because BonsaiWorldStateKeyValueStorage is persistent + // No need to close or notify because BonsaiWorldStateKeyValueStorage is persistent } public interface BonsaiUpdater extends WorldStateStorage.Updater { @@ -439,4 +435,12 @@ public void rollback() { trieLogStorageTransaction.rollback(); } } + + interface BonsaiStorageSubscriber { + default void onClearStorage() {} + + default void onClearFlatDatabaseStorage() {} + + default void onCloseStorage() {} + } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/CachedMerkleTrieLoader.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/CachedMerkleTrieLoader.java index 138891956491..b2bfdd4d2c69 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/CachedMerkleTrieLoader.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/CachedMerkleTrieLoader.java @@ -17,6 +17,7 @@ import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.bonsai.BonsaiWorldStateKeyValueStorage.BonsaiStorageSubscriber; import org.hyperledger.besu.ethereum.trie.MerklePatriciaTrie; import org.hyperledger.besu.ethereum.trie.MerkleTrieException; import org.hyperledger.besu.ethereum.trie.StoredMerklePatriciaTrie; @@ -35,7 +36,7 @@ import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; -public class CachedMerkleTrieLoader { +public class CachedMerkleTrieLoader implements BonsaiStorageSubscriber { private static final int ACCOUNT_CACHE_SIZE = 100_000; private static final int STORAGE_CACHE_SIZE = 200_000; @@ -67,7 +68,7 @@ public void cacheAccountNodes( final BonsaiWorldStateKeyValueStorage worldStateStorage, final Hash worldStateRootHash, final Address account) { - final long worldStateSubscriberId = worldStateStorage.subscribe(); + final long storageSubscriberId = worldStateStorage.subscribe(this); try { final StoredMerklePatriciaTrie accountTrie = new StoredMerklePatriciaTrie<>( @@ -83,7 +84,7 @@ public void cacheAccountNodes( } catch (MerkleTrieException e) { // ignore exception for the cache } finally { - worldStateStorage.unSubscribe(worldStateSubscriberId); + worldStateStorage.unSubscribe(storageSubscriberId); } } @@ -100,7 +101,7 @@ public void cacheStorageNodes( final Address account, final Hash slotHash) { final Hash accountHash = Hash.hash(account); - final long worldStateSubscriberId = worldStateStorage.subscribe(); + final long storageSubscriberId = worldStateStorage.subscribe(this); try { worldStateStorage .getStateTrieNode(Bytes.concatenate(accountHash, Bytes.EMPTY)) @@ -125,7 +126,7 @@ public void cacheStorageNodes( } }); } finally { - worldStateStorage.unSubscribe(worldStateSubscriberId); + worldStateStorage.unSubscribe(storageSubscriberId); } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/LayeredTrieLogManager.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/LayeredTrieLogManager.java index 83c72516d44b..ca972f1a71d0 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/LayeredTrieLogManager.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/LayeredTrieLogManager.java @@ -28,15 +28,14 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class LayeredTrieLogManager - extends AbstractTrieLogManager { +public class LayeredTrieLogManager extends AbstractTrieLogManager { private static final Logger LOG = LoggerFactory.getLogger(LayeredTrieLogManager.class); - public LayeredTrieLogManager( + LayeredTrieLogManager( final Blockchain blockchain, final BonsaiWorldStateKeyValueStorage worldStateStorage, final long maxLayersToLoad, - final Map cachedWorldStatesByHash) { + final Map> cachedWorldStatesByHash) { super(blockchain, worldStateStorage, maxLayersToLoad, cachedWorldStatesByHash); } @@ -47,23 +46,19 @@ public LayeredTrieLogManager( this(blockchain, worldStateStorage, maxLayersToLoad, new HashMap<>()); } - public LayeredTrieLogManager( - final Blockchain blockchain, final BonsaiWorldStateKeyValueStorage worldStateStorage) { - this(blockchain, worldStateStorage, RETAINED_LAYERS, new HashMap<>()); - } - @Override public synchronized void addCachedLayer( final BlockHeader blockHeader, final Hash worldStateRootHash, final TrieLogLayer trieLog, - final BonsaiWorldStateArchive worldStateArchive) { + final BonsaiWorldStateArchive worldStateArchive, + final BonsaiPersistedWorldState forWorldState) { final BonsaiLayeredWorldState bonsaiLayeredWorldState = new BonsaiLayeredWorldState( blockchain, worldStateArchive, - Optional.of((BonsaiPersistedWorldState) worldStateArchive.getMutable()), + Optional.of(forWorldState), blockHeader.getNumber(), worldStateRootHash, trieLog); @@ -71,7 +66,7 @@ public synchronized void addCachedLayer( LOG, "adding layered world state for block {}, state root hash {}", blockHeader::toLogString, - worldStateRootHash::toHexString); + worldStateRootHash::toShortHexString); cachedWorldStatesByHash.put( blockHeader.getHash(), new LayeredWorldStateCache(bonsaiLayeredWorldState)); } @@ -91,7 +86,7 @@ public synchronized void updateCachedLayers(final Hash blockParentHash, final Ha }); } - public static class LayeredWorldStateCache implements CachedWorldState { + public static class LayeredWorldStateCache implements CachedWorldState { final BonsaiLayeredWorldState layeredWorldState; @@ -99,6 +94,11 @@ public LayeredWorldStateCache(final BonsaiLayeredWorldState layeredWorldState) { this.layeredWorldState = layeredWorldState; } + @Override + public void dispose() { + // no-op + } + @Override public long getHeight() { return layeredWorldState.getHeight(); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/SnapshotTrieLogManager.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/SnapshotTrieLogManager.java index e7f6bd87e349..45c1effe3197 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/SnapshotTrieLogManager.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/SnapshotTrieLogManager.java @@ -18,95 +18,121 @@ import static org.hyperledger.besu.util.Slf4jLambdaHelper.debugLambda; import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.bonsai.BonsaiWorldStateKeyValueStorage.BonsaiStorageSubscriber; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.MutableWorldState; -import java.util.HashMap; import java.util.Map; import java.util.Optional; -import java.util.function.Supplier; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; -import com.google.common.base.Suppliers; import org.apache.tuweni.bytes.Bytes32; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class SnapshotTrieLogManager - extends AbstractTrieLogManager { +public class SnapshotTrieLogManager extends AbstractTrieLogManager + implements BonsaiStorageSubscriber { private static final Logger LOG = LoggerFactory.getLogger(SnapshotTrieLogManager.class); public SnapshotTrieLogManager( final Blockchain blockchain, final BonsaiWorldStateKeyValueStorage worldStateStorage, final long maxLayersToLoad) { - this(blockchain, worldStateStorage, maxLayersToLoad, new HashMap<>()); + this(blockchain, worldStateStorage, maxLayersToLoad, new ConcurrentHashMap<>()); } - public SnapshotTrieLogManager( + SnapshotTrieLogManager( final Blockchain blockchain, final BonsaiWorldStateKeyValueStorage worldStateStorage, final long maxLayersToLoad, - final Map cachedWorldStatesByHash) { + final Map> cachedWorldStatesByHash) { super(blockchain, worldStateStorage, maxLayersToLoad, cachedWorldStatesByHash); + worldStateStorage.subscribe(this); } @Override - public void addCachedLayer( + protected void addCachedLayer( final BlockHeader blockHeader, final Hash worldStateRootHash, final TrieLogLayer trieLog, - final BonsaiWorldStateArchive worldStateArchive) { + final BonsaiWorldStateArchive worldStateArchive, + final BonsaiPersistedWorldState worldState) { debugLambda( LOG, "adding snapshot world state for block {}, state root hash {}", blockHeader::toLogString, - worldStateRootHash::toHexString); + worldStateRootHash::toShortHexString); + + // TODO: add a generic param so we don't have to cast: + BonsaiSnapshotWorldState snapshotWorldState; + if (worldState instanceof BonsaiSnapshotWorldState) { + snapshotWorldState = (BonsaiSnapshotWorldState) worldState; + } else { + snapshotWorldState = + BonsaiSnapshotWorldState.create(worldStateArchive, rootWorldStateStorage); + } + cachedWorldStatesByHash.put( blockHeader.getHash(), - new CachedSnapshotWorldState( - () -> - worldStateArchive - .getMutableSnapshot(blockHeader.getHash()) - .map(BonsaiSnapshotWorldState.class::cast) - .orElse(null), - trieLog, - blockHeader.getNumber())); + new CachedSnapshotWorldState(snapshotWorldState, trieLog, blockHeader.getNumber())); + } + + @Override + public void updateCachedLayers(final Hash blockParentHash, final Hash blockHash) { + // no-op. } @Override - public Optional getBonsaiCachedWorldState(final Hash blockHash) { + public synchronized Optional getBonsaiCachedWorldState(final Hash blockHash) { if (cachedWorldStatesByHash.containsKey(blockHash)) { return Optional.ofNullable(cachedWorldStatesByHash.get(blockHash)) - .map(CachedSnapshotWorldState::getMutableWorldState) + .map(CachedWorldState::getMutableWorldState) .map(MutableWorldState::copy); } return Optional.empty(); } @Override - public void updateCachedLayers(final Hash blockParentHash, final Hash blockHash) { - // fetch the snapshot supplier as soon as its block has been added: - Optional.ofNullable(cachedWorldStatesByHash.get(blockHash)) - .ifPresent(CachedSnapshotWorldState::getMutableWorldState); + public synchronized void onClearStorage() { + dropArchive(); } - public static class CachedSnapshotWorldState implements CachedWorldState { + @Override + public synchronized void onClearFlatDatabaseStorage() { + dropArchive(); + } + + private void dropArchive() { + // drop all cached snapshot worldstates, they are unsafe when the db has been truncated + LOG.info("Key-value storage truncated, dropping cached worldstates"); + cachedWorldStatesByHash.clear(); + } - final Supplier snapshot; + public static class CachedSnapshotWorldState + implements CachedWorldState, BonsaiStorageSubscriber { + + final BonsaiSnapshotWorldState snapshot; + final Long snapshotSubscriberId; final TrieLogLayer trieLog; final long height; + final AtomicBoolean isClosed = new AtomicBoolean(false); public CachedSnapshotWorldState( - final Supplier snapshotSupplier, - final TrieLogLayer trieLog, - final long height) { - this.snapshot = Suppliers.memoize(snapshotSupplier::get); + final BonsaiSnapshotWorldState snapshot, final TrieLogLayer trieLog, final long height) { + this.snapshotSubscriberId = snapshot.getWorldStateStorage().subscribe(this); + this.snapshot = snapshot; this.trieLog = trieLog; this.height = height; } + @Override + public void dispose() { + snapshot.worldStateStorage.unSubscribe(snapshotSubscriberId); + } + @Override public long getHeight() { return height; @@ -118,8 +144,11 @@ public TrieLogLayer getTrieLog() { } @Override - public MutableWorldState getMutableWorldState() { - return snapshot.get(); + public synchronized BonsaiSnapshotWorldState getMutableWorldState() { + if (isClosed.get()) { + return null; + } + return snapshot; } } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/TrieLogManager.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/TrieLogManager.java index b107513a9c06..31720cc73b07 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/TrieLogManager.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/TrieLogManager.java @@ -26,28 +26,25 @@ public interface TrieLogManager { void saveTrieLog( final BonsaiWorldStateArchive worldStateArchive, final BonsaiWorldStateUpdater localUpdater, - final Hash worldStateRootHash, - final BlockHeader blockHeader); + final Hash forWorldStateRootHash, + final BlockHeader forBlockHeader, + final BonsaiPersistedWorldState forWorldState); Optional getBonsaiCachedWorldState(final Hash blockHash); long getMaxLayersToLoad(); - void addCachedLayer( - final BlockHeader blockHeader, - final Hash worldStateRootHash, - final TrieLogLayer trieLog, - final BonsaiWorldStateArchive worldStateArchive); - void updateCachedLayers(final Hash blockParentHash, final Hash blockHash); Optional getTrieLogLayer(final Hash blockHash); - interface CachedWorldState { + interface CachedWorldState { + void dispose(); + long getHeight(); TrieLogLayer getTrieLog(); - MutableWorldState getMutableWorldState(); + Z getMutableWorldState(); } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/SnapshotMutableWorldState.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/SnapshotMutableWorldState.java index 611eaa5e9a3d..941373f44069 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/SnapshotMutableWorldState.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/SnapshotMutableWorldState.java @@ -15,4 +15,8 @@ */ package org.hyperledger.besu.ethereum.core; -public interface SnapshotMutableWorldState extends MutableWorldState, AutoCloseable {} +import org.hyperledger.besu.ethereum.bonsai.BonsaiWorldStateKeyValueStorage; + +public interface SnapshotMutableWorldState extends MutableWorldState, AutoCloseable { + BonsaiWorldStateKeyValueStorage getWorldStateStorage(); +} diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/AbstractIsolationTests.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/AbstractIsolationTests.java index 27e52e186639..8b2e3a09753a 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/AbstractIsolationTests.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/AbstractIsolationTests.java @@ -73,6 +73,7 @@ public abstract class AbstractIsolationTests { protected BonsaiWorldStateArchive archive; + protected BonsaiWorldStateKeyValueStorage bonsaiWorldStateStorage; protected ProtocolContext protocolContext; final Function asKeyPair = key -> @@ -107,11 +108,12 @@ protected boolean shouldUseSnapshots() { @Before public void createStorage() { - // final InMemoryKeyValueStorageProvider provider = new InMemoryKeyValueStorageProvider(); + bonsaiWorldStateStorage = + (BonsaiWorldStateKeyValueStorage) + createKeyValueStorageProvider().createWorldStateStorage(DataStorageFormat.BONSAI); archive = new BonsaiWorldStateArchive( - (BonsaiWorldStateKeyValueStorage) - createKeyValueStorageProvider().createWorldStateStorage(DataStorageFormat.BONSAI), + bonsaiWorldStateStorage, blockchain, Optional.of(16L), shouldUseSnapshots(), diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotIsolationTests.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotIsolationTests.java index 7e22b6a8cbe6..ee2c34e81ba5 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotIsolationTests.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotIsolationTests.java @@ -33,6 +33,21 @@ @RunWith(MockitoJUnitRunner.class) public class BonsaiSnapshotIsolationTests extends AbstractIsolationTests { + @Test + public void ensureTruncateDoesNotCauseSegfault() { + + var preTruncatedWorldState = archive.getMutable(null, genesisState.getBlock().getHash(), false); + assertThat(preTruncatedWorldState) + .isPresent(); // really just assert that we have not segfaulted after truncating + bonsaiWorldStateStorage.clear(); + var postTruncatedWorldState = + archive.getMutable(null, genesisState.getBlock().getHash(), false); + assertThat(postTruncatedWorldState).isEmpty(); + // assert that trying to access pre-worldstate does not segfault after truncating + preTruncatedWorldState.get().get(Address.fromHexString(accounts.get(0).getAddress())); + assertThat(true).isTrue(); + } + @Test public void testIsolatedFromHead_behindHead() { Address testAddress = Address.fromHexString("0xdeadbeef"); diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldStateArchiveTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldStateArchiveTest.java index 9802cc99ca74..a72ba7f4f287 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldStateArchiveTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldStateArchiveTest.java @@ -28,6 +28,7 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.bonsai.SnapshotTrieLogManager.CachedSnapshotWorldState; +import org.hyperledger.besu.ethereum.bonsai.TrieLogManager.CachedWorldState; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; @@ -123,20 +124,15 @@ public void testGetMutableWithRollbackNotOverrideTrieLogLayer() { final BlockHeader blockHeaderChainB = blockBuilder.number(1).timestamp(2).parentHash(genesis.getHash()).buildHeader(); - final Map worldStatesByHash = mock(HashMap.class); - when(worldStatesByHash.containsKey(any(Bytes32.class))).thenReturn(true); - when(worldStatesByHash.get(eq(blockHeaderChainA.getHash()))) - .thenReturn( - new CachedSnapshotWorldState( - () -> mock(BonsaiSnapshotWorldState.class, Answers.RETURNS_MOCKS), - mock(TrieLogLayer.class), - 2)); - when(worldStatesByHash.get(eq(blockHeaderChainB.getHash()))) - .thenReturn( - new CachedSnapshotWorldState( - () -> mock(BonsaiSnapshotWorldState.class, Answers.RETURNS_MOCKS), - mock(TrieLogLayer.class), - 2)); + final Map> worldStatesByHash = + new HashMap<>(); + var mockCachedState = + new CachedSnapshotWorldState( + mock(BonsaiSnapshotWorldState.class, Answers.RETURNS_MOCKS), + mock(TrieLogLayer.class, Answers.RETURNS_MOCKS), + 2); + worldStatesByHash.put(blockHeaderChainA.getHash(), mockCachedState); + worldStatesByHash.put(blockHeaderChainB.getHash(), mockCachedState); var worldStateStorage = new BonsaiWorldStateKeyValueStorage(storageProvider); bonsaiWorldStateArchive = spy( diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchiveTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchiveTest.java index 56676ec21450..ac70222af640 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchiveTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchiveTest.java @@ -29,6 +29,7 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.bonsai.LayeredTrieLogManager.LayeredWorldStateCache; +import org.hyperledger.besu.ethereum.bonsai.TrieLogManager.CachedWorldState; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; @@ -208,7 +209,8 @@ public void testGetMutableWithStorageConsistencyToRollbackAndRollForwardTheState final BlockHeader blockHeaderChainB = blockBuilder.number(1).timestamp(2).parentHash(genesis.getHash()).buildHeader(); - final Map layeredWorldStatesByHash = mock(HashMap.class); + final Map> layeredWorldStatesByHash = + mock(HashMap.class); when(layeredWorldStatesByHash.containsKey(any(Bytes32.class))).thenReturn(true); when(layeredWorldStatesByHash.get(eq(blockHeaderChainA.getHash()))) .thenReturn( @@ -262,7 +264,8 @@ public void testGetMutableWithRollbackNotOverrideTrieLogLayer() { final BlockHeader blockHeaderChainB = blockBuilder.number(1).timestamp(2).parentHash(genesis.getHash()).buildHeader(); - final Map layeredWorldStatesByHash = mock(HashMap.class); + final Map> layeredWorldStatesByHash = + mock(HashMap.class); when(layeredWorldStatesByHash.containsKey(any(Bytes32.class))).thenReturn(true); when(layeredWorldStatesByHash.get(eq(blockHeaderChainA.getHash()))) .thenReturn( diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/LayeredWorldStateTests.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/LayeredWorldStateTests.java new file mode 100644 index 000000000000..2bdbeb5465fa --- /dev/null +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/LayeredWorldStateTests.java @@ -0,0 +1,131 @@ +/* + * Copyright Hyperledger Besu contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * + */ +package org.hyperledger.besu.ethereum.bonsai; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.ethereum.chain.Blockchain; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; +import org.hyperledger.besu.ethereum.core.SnapshotMutableWorldState; + +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.apache.tuweni.bytes.Bytes; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Answers; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class LayeredWorldStateTests { + + @Mock BonsaiWorldStateArchive archive; + @Mock Blockchain blockchain; + + @Test + public void layeredWorldStateUsesCorrectPersistedWorldStateOnCopy() { + // when copying a layered worldstate we return mutable copy, + // ensure it is for the correct/corresponding worldstate: + + Hash state1Hash = Hash.hash(Bytes.of("first_state".getBytes(StandardCharsets.UTF_8))); + Hash block1Hash = Hash.hash(Bytes.of("first_block".getBytes(StandardCharsets.UTF_8))); + var mockStorage = mock(BonsaiWorldStateKeyValueStorage.class); + when(mockStorage.getWorldStateBlockHash()).thenReturn(Optional.of(block1Hash)); + when(mockStorage.getWorldStateRootHash()).thenReturn(Optional.of(state1Hash)); + SnapshotMutableWorldState mockState = + when(mock(SnapshotMutableWorldState.class).getWorldStateStorage()) + .thenReturn(mockStorage) + .getMock(); + + TrieLogLayer mockLayer = + when(mock(TrieLogLayer.class).getBlockHash()).thenReturn(Hash.ZERO).getMock(); + BonsaiLayeredWorldState mockLayerWs = + new BonsaiLayeredWorldState( + blockchain, + archive, + Optional.of(mock(BonsaiLayeredWorldState.class)), + 1L, + state1Hash, + mockLayer); + + // mimic persisted state being at a different state: + when(archive.getMutableSnapshot(mockLayer.getBlockHash())).thenReturn(Optional.of(mockState)); + + try (var copyOfLayer1 = mockLayerWs.copy()) { + assertThat(copyOfLayer1.rootHash()).isEqualTo(state1Hash); + } catch (Exception ex) { + throw new RuntimeException(ex); + } + } + + @Test + public void saveTrieLogShouldUseCorrectPersistedWorldStateOnCopy() { + // when we save a snapshot based worldstate, ensure + // we used the passed in worldstate and roothash for calculating the trielog diff + Hash testStateRoot = Hash.fromHexStringLenient("0xdeadbeef"); + BlockHeader testHeader = new BlockHeaderTestFixture().stateRoot(testStateRoot).buildHeader(); + + BonsaiWorldStateKeyValueStorage testStorage = + mock(BonsaiWorldStateKeyValueStorage.class, Answers.RETURNS_DEEP_STUBS); + + BonsaiSnapshotWorldState testState = mock(BonsaiSnapshotWorldState.class); + when(testState.getWorldStateStorage()).thenReturn(testStorage); + when(testState.rootHash()).thenReturn(testStateRoot); + when(testState.blockHash()).thenReturn(testHeader.getBlockHash()); + + BonsaiWorldStateUpdater testUpdater = new BonsaiWorldStateUpdater(testState); + // mock kvstorage to mimic head being in a different state than testState + LayeredTrieLogManager manager = + spy( + new LayeredTrieLogManager( + blockchain, mock(BonsaiWorldStateKeyValueStorage.class), 10L, new HashMap<>())); + + // assert we are using the target worldstate storage: + final AtomicBoolean calledPrepareTrieLog = new AtomicBoolean(false); + doAnswer( + prepareCallSpec -> { + Hash blockHash = prepareCallSpec.getArgument(0, BlockHeader.class).getHash(); + Hash rootHash = prepareCallSpec.getArgument(1, Hash.class); + BonsaiPersistedWorldState ws = + prepareCallSpec.getArgument(4, BonsaiPersistedWorldState.class); + assertThat(ws.rootHash()).isEqualTo(rootHash); + assertThat(ws.blockHash()).isEqualTo(blockHash); + calledPrepareTrieLog.set(true); + return mock(TrieLogLayer.class); + }) + .when(manager) + .prepareTrieLog( + any(BlockHeader.class), + any(Hash.class), + any(BonsaiWorldStateUpdater.class), + any(BonsaiWorldStateArchive.class), + any(BonsaiPersistedWorldState.class)); + + manager.saveTrieLog(archive, testUpdater, testStateRoot, testHeader, testState); + assertThat(calledPrepareTrieLog.get()).isTrue(); + } +} diff --git a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBSnapshot.java b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBSnapshot.java index 7d75d3d95678..7fc2d048cf87 100644 --- a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBSnapshot.java +++ b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBSnapshot.java @@ -36,12 +36,12 @@ class RocksDBSnapshot { this.dbSnapshot = db.getSnapshot(); } - Snapshot markAndUseSnapshot() { + synchronized Snapshot markAndUseSnapshot() { usages.incrementAndGet(); return dbSnapshot; } - void unMarkSnapshot() { + synchronized void unMarkSnapshot() { if (usages.decrementAndGet() < 1) { db.releaseSnapshot(dbSnapshot); dbSnapshot.close(); diff --git a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBSnapshotTransaction.java b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBSnapshotTransaction.java index 659b690ad280..c7a528d669fc 100644 --- a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBSnapshotTransaction.java +++ b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBSnapshotTransaction.java @@ -22,6 +22,7 @@ import org.hyperledger.besu.plugin.services.storage.rocksdb.RocksDbIterator; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Stream; import org.apache.commons.lang3.tuple.Pair; @@ -46,6 +47,7 @@ public class RocksDBSnapshotTransaction implements KeyValueStorageTransaction, A private final RocksDBSnapshot snapshot; private final WriteOptions writeOptions; private final ReadOptions readOptions; + private final AtomicBoolean isClosed = new AtomicBoolean(false); RocksDBSnapshotTransaction( final OptimisticTransactionDB db, @@ -77,6 +79,11 @@ private RocksDBSnapshotTransaction( } public Optional get(final byte[] key) { + if (isClosed.get()) { + LOG.debug("Attempted to access closed snapshot"); + return Optional.empty(); + } + try (final OperationTimer.TimingContext ignored = metrics.getReadLatency().startTimer()) { return Optional.ofNullable(snapTx.get(columnFamilyHandle, readOptions, key)); } catch (final RocksDBException e) { @@ -86,6 +93,11 @@ public Optional get(final byte[] key) { @Override public void put(final byte[] key, final byte[] value) { + if (isClosed.get()) { + LOG.debug("Attempted to access closed snapshot"); + return; + } + try (final OperationTimer.TimingContext ignored = metrics.getWriteLatency().startTimer()) { snapTx.put(columnFamilyHandle, key, value); } catch (final RocksDBException e) { @@ -99,6 +111,10 @@ public void put(final byte[] key, final byte[] value) { @Override public void remove(final byte[] key) { + if (isClosed.get()) { + LOG.debug("Attempted to access closed snapshot"); + return; + } try (final OperationTimer.TimingContext ignored = metrics.getRemoveLatency().startTimer()) { snapTx.delete(columnFamilyHandle, key); } catch (final RocksDBException e) { @@ -145,6 +161,9 @@ public void rollback() { } public RocksDBSnapshotTransaction copy() { + if (isClosed.get()) { + throw new StorageException("Snapshot already closed"); + } try { var copyReadOptions = new ReadOptions().setSnapshot(snapshot.markAndUseSnapshot()); var copySnapTx = db.beginTransaction(writeOptions); @@ -164,5 +183,6 @@ public void close() { writeOptions.close(); readOptions.close(); snapshot.unMarkSnapshot(); + isClosed.set(true); } }