Merge pull request #5764 from multiversx/fix-snapshots-manager

Fix snapshots manager

common/interface.go

@@ -110,6 +110,7 @@ type StorageManager interface {
 	RemoveFromAllActiveEpochs(hash []byte) error
 	SetEpochForPutOperation(uint32)
 	ShouldTakeSnapshot() bool
+	IsSnapshotSupported() bool
 	GetBaseTrieStorageManager() StorageManager
 	IsClosed() bool
 	Close() error

state/snapshotsManager.go

@@ -155,6 +155,15 @@ func (sm *snapshotsManager) SnapshotState(
 	epoch uint32,
 	trieStorageManager common.StorageManager,
 ) {
+	if check.IfNil(trieStorageManager) {
+		return
+	}
+	if !trieStorageManager.IsSnapshotSupported() {
+		log.Debug("skipping snapshot as the snapshot is not supported by the current trieStorageManager",
+			"trieStorageManager type", fmt.Sprintf("%T", trieStorageManager))
+		return
+	}
+
 	sm.mutex.Lock()
 
 	stats, skipSnapshot := sm.prepareSnapshot(rootHash, epoch, trieStorageManager)

state/snapshotsManager_test.go

@@ -2,6 +2,7 @@ package state_test
 
 import (
 	"errors"
+	"fmt"
 	"sync"
 	"testing"
 	"time"
@@ -236,6 +237,50 @@ func TestSnapshotsManager_SnapshotState(t *testing.T) {
 	rootHash := []byte("rootHash")
 	epoch := uint32(5)
 
+	t.Run("nil snapshots manager should not panic", func(t *testing.T) {
+		t.Parallel()
+
+		defer func() {
+			r := recover()
+			if r != nil {
+				assert.Fail(t, fmt.Sprintf("should have not panicked %v", r))
+			}
+		}()
+
+		args := getDefaultSnapshotManagerArgs()
+		sm, _ := state.NewSnapshotsManager(args)
+		sm.SnapshotState(rootHash, epoch, nil)
+	})
+	t.Run("storage manager does not support snapshots should not initiate snapshotting", func(t *testing.T) {
+		t.Parallel()
+
+		args := getDefaultSnapshotManagerArgs()
+		args.StateMetrics = &stateTest.StateMetricsStub{
+			GetSnapshotMessageCalled: func() string {
+				assert.Fail(t, "should have not called GetSnapshotMessage")
+				return ""
+			},
+		}
+		sm, _ := state.NewSnapshotsManager(args)
+		tsm := &storageManager.StorageManagerStub{
+			PutInEpochCalled: func(key []byte, val []byte, e uint32) error {
+				assert.Fail(t, "should have not called put in epoch")
+				return nil
+			},
+			EnterPruningBufferingModeCalled: func() {
+				assert.Fail(t, "should have not called enter pruning buffering mode")
+			},
+			IsSnapshotSupportedCalled: func() bool {
+				return false
+			},
+		}
+
+		sm.SnapshotState(rootHash, epoch, tsm)
+
+		lastRootHash, lastEpoch := sm.GetLastSnapshotInfo()
+		assert.Nil(t, lastRootHash)
+		assert.Zero(t, lastEpoch)
+	})
 	t.Run("should not start snapshot for same rootHash in same epoch, and lastSnapshot should not be rewritten", func(t *testing.T) {
 		t.Parallel()
 

state/storagePruningManager/evictionWaitingList/memoryEvictionWaitingList.go

@@ -61,7 +61,7 @@ func (mewl *memoryEvictionWaitingList) Put(rootHash []byte, hashes common.Modifi
 	mewl.opMutex.Lock()
 	defer mewl.opMutex.Unlock()
 
-	log.Trace("trie eviction waiting list", "size", len(mewl.cache))
+	log.Trace("trie eviction waiting list", "cache size", len(mewl.cache), "reversed cache size", len(mewl.reversedCache))
 
 	mewl.putInReversedCache(rootHash, hashes)
 	mewl.putInCache(rootHash, hashes)
@@ -158,7 +158,7 @@ func (mewl *memoryEvictionWaitingList) Evict(rootHash []byte) (common.ModifiedHa
 
 	rhData, ok := mewl.cache[string(rootHash)]
 	if !ok {
-		return make(common.ModifiedHashes, 0), nil
+		return make(common.ModifiedHashes), nil
 	}
 
 	if rhData.numReferences <= 1 {
@@ -170,7 +170,7 @@ func (mewl *memoryEvictionWaitingList) Evict(rootHash []byte) (common.ModifiedHa
 
 	rhData.numReferences--
 
-	return make(common.ModifiedHashes, 0), nil
+	return make(common.ModifiedHashes), nil
 }
 
 // IsInterfaceNil returns true if there is no value under the interface

testscommon/storageManager/storageManagerStub.go

@@ -31,6 +31,7 @@ type StorageManagerStub struct {
 	GetIdentifierCalled                    func() string
 	CloseCalled                            func() error
 	RemoveFromAllActiveEpochsCalled        func(hash []byte) error
+	IsSnapshotSupportedCalled              func() bool
 }
 
 // Put -
@@ -238,6 +239,15 @@ func (sms *StorageManagerStub) GetIdentifier() string {
 	return ""
 }
 
+// IsSnapshotSupported -
+func (sms *StorageManagerStub) IsSnapshotSupported() bool {
+	if sms.IsSnapshotSupportedCalled != nil {
+		return sms.IsSnapshotSupportedCalled()
+	}
+
+	return true
+}
+
 // IsInterfaceNil -
 func (sms *StorageManagerStub) IsInterfaceNil() bool {
 	return sms == nil

trie/trieStorageManager.go

@@ -666,6 +666,11 @@ func (tsm *trieStorageManager) ShouldTakeSnapshot() bool {
 	return true
 }
 
+// IsSnapshotSupported returns true as the snapshotting process is supported by the current implementation
+func (tsm *trieStorageManager) IsSnapshotSupported() bool {
+	return true
+}
+
 func isTrieSynced(stsm *snapshotTrieStorageManager) bool {
 	val, err := stsm.GetFromCurrentEpoch([]byte(common.TrieSyncedKey))
 	if err != nil {

trie/trieStorageManagerWithoutSnapshot.go

@@ -57,6 +57,11 @@ func (tsm *trieStorageManagerWithoutSnapshot) ShouldTakeSnapshot() bool {
 	return false
 }
 
+// IsSnapshotSupported returns false as the snapshotting process is not supported by the current implementation
+func (tsm *trieStorageManagerWithoutSnapshot) IsSnapshotSupported() bool {
+	return false
+}
+
 // IsInterfaceNil returns true if there is no value under the interface
 func (tsm *trieStorageManagerWithoutSnapshot) IsInterfaceNil() bool {
 	return tsm == nil

trie/trieStorageManagerWithoutSnapshot_test.go

@@ -146,3 +146,13 @@ func TestTrieStorageManagerWithoutSnapshot_IsInterfaceNil(t *testing.T) {
 	ts, _ = trie.NewTrieStorageManagerWithoutSnapshot(tsm)
 	assert.False(t, check.IfNil(ts))
 }
+
+func TestTrieStorageManagerWithoutSnapshot_IsSnapshotSupportedShouldReturnFalse(t *testing.T) {
+	t.Parallel()
+
+	args := trie.GetDefaultTrieStorageManagerParameters()
+	tsm, _ := trie.NewTrieStorageManager(args)
+	ts, _ := trie.NewTrieStorageManagerWithoutSnapshot(tsm)
+
+	assert.False(t, ts.IsSnapshotSupported())
+}

trie/trieStorageManager_test.go

@@ -902,3 +902,12 @@ func TestTrieStorageManager_GetIdentifier(t *testing.T) {
 	id := ts.GetIdentifier()
 	assert.Equal(t, expectedId, id)
 }
+
+func TestTrieStorageManager_IsSnapshotSupportedShouldReturnTrue(t *testing.T) {
+	t.Parallel()
+
+	args := trie.GetDefaultTrieStorageManagerParameters()
+	ts, _ := trie.NewTrieStorageManager(args)
+
+	assert.True(t, ts.IsSnapshotSupported())
+}