Clarify break on error during ancestors lookup (ava-labs#1580)

joshua-kim · Jun 7, 2023 · 110bb61 · 110bb61
1 parent 9026e30
commit 110bb61
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 14 deletions.
diff --git a/snow/engine/snowman/block/batched_vm.go b/snow/engine/snowman/block/batched_vm.go
@@ -8,9 +8,12 @@ import (
 	"errors"
 	"time"
 
+	"go.uber.org/zap"
+
 	"github.com/ava-labs/avalanchego/database"
 	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/snow/consensus/snowman"
+	"github.com/ava-labs/avalanchego/utils/logging"
 	"github.com/ava-labs/avalanchego/utils/wrappers"
 )
 
@@ -33,6 +36,7 @@ type BatchedChainVM interface {
 
 func GetAncestors(
 	ctx context.Context,
+	log logging.Logger,
 	vm Getter, // fetch blocks
 	blkID ids.ID, // first requested block
 	maxBlocksNum int, // max number of blocks to be retrieved
@@ -60,7 +64,7 @@ func GetAncestors(
 	startTime := time.Now()
 	blk, err := vm.GetBlock(ctx, blkID)
 	if err == database.ErrNotFound {
-		// special case ErrNotFound as an empty response: this signals
+		// Special case ErrNotFound as an empty response: this signals
 		// the client to avoid contacting this node for further ancestors
 		// as they may have been pruned or unavailable due to state-sync.
 		return nil, nil
@@ -74,8 +78,17 @@ func GetAncestors(
 	ancestorsBytesLen := len(blk.Bytes()) + wrappers.IntLen // length, in bytes, of all elements of ancestors
 
 	for numFetched := 1; numFetched < maxBlocksNum && time.Since(startTime) < maxBlocksRetrivalTime; numFetched++ {
-		blk, err = vm.GetBlock(ctx, blk.Parent())
+		parentID := blk.Parent()
+		blk, err = vm.GetBlock(ctx, parentID)
+		if err == database.ErrNotFound {
+			// After state sync we may not have the full chain
+			break
+		}
 		if err != nil {
+			log.Error("failed to get block during ancestors lookup",
+				zap.String("parentID", parentID.String()),
+				zap.Error(err),
+			)
 			break
 		}
 		blkBytes := blk.Bytes()
@@ -84,7 +97,7 @@ func GetAncestors(
 		// is repr. by an int.
 		newLen := ancestorsBytesLen + len(blkBytes) + wrappers.IntLen
 		if newLen > maxBlocksSize {
-			// reached maximum response size
+			// Reached maximum response size
 			break
 		}
 		ancestorsBytes = append(ancestorsBytes, blkBytes)

diff --git a/snow/engine/snowman/block/batched_vm_test.go b/snow/engine/snowman/block/batched_vm_test.go
@@ -14,6 +14,7 @@ import (
 	"github.com/ava-labs/avalanchego/database"
 	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/snow/consensus/snowman"
+	"github.com/ava-labs/avalanchego/utils/logging"
 )
 
 var errTest = errors.New("non-nil error")
@@ -25,7 +26,7 @@ func TestGetAncestorsDatabaseNotFound(t *testing.T) {
 		require.Equal(t, someID, id)
 		return nil, database.ErrNotFound
 	}
-	containers, err := GetAncestors(context.Background(), vm, someID, 10, 10, 1*time.Second)
+	containers, err := GetAncestors(context.Background(), logging.NoLog{}, vm, someID, 10, 10, 1*time.Second)
 	require.NoError(t, err)
 	require.Empty(t, containers)
 }
@@ -39,7 +40,7 @@ func TestGetAncestorsPropagatesErrors(t *testing.T) {
 		require.Equal(t, someID, id)
 		return nil, errTest
 	}
-	containers, err := GetAncestors(context.Background(), vm, someID, 10, 10, 1*time.Second)
+	containers, err := GetAncestors(context.Background(), logging.NoLog{}, vm, someID, 10, 10, 1*time.Second)
 	require.Nil(t, containers)
 	require.ErrorIs(t, err, errTest)
 }
diff --git a/snow/engine/snowman/getter/getter.go b/snow/engine/snowman/getter/getter.go
@@ -150,6 +150,7 @@ func (gh *getter) GetAccepted(ctx context.Context, nodeID ids.NodeID, requestID
 func (gh *getter) GetAncestors(ctx context.Context, nodeID ids.NodeID, requestID uint32, blkID ids.ID) error {
 	ancestorsBytes, err := block.GetAncestors(
 		ctx,
+		gh.log,
 		gh.vm,
 		blkID,
 		gh.cfg.AncestorsMaxContainersSent,

diff --git a/vms/rpcchainvm/vm_server.go b/vms/rpcchainvm/vm_server.go
@@ -75,6 +75,7 @@ type VMServer struct {
 
 	processMetrics prometheus.Gatherer
 	dbManager      manager.Manager
+	log            logging.Logger
 
 	serverCloser grpcutils.ServerCloser
 	connCloser   wrappers.Closer
@@ -184,6 +185,16 @@ func (vm *VMServer) Initialize(ctx context.Context, req *vmpb.InitializeRequest)
 	}
 	vm.dbManager = dbManager
 
+	// TODO: Allow the logger to be configured by the client
+	vm.log = logging.NewLogger(
+		fmt.Sprintf("<%s Chain>", chainID),
+		logging.NewWrappedCore(
+			logging.Info,
+			originalStderr,
+			logging.Colors.ConsoleEncoder(),
+		),
+	)
+
 	clientConn, err := grpcutils.Dial(
 		req.ServerAddr,
 		grpcutils.WithChainUnaryInterceptor(grpcClientMetrics.UnaryClientInterceptor()),
@@ -233,15 +244,7 @@ func (vm *VMServer) Initialize(ctx context.Context, req *vmpb.InitializeRequest)
 		CChainID:    cChainID,
 		AVAXAssetID: avaxAssetID,
 
-		// TODO: Allow the logger to be configured by the client
-		Log: logging.NewLogger(
-			fmt.Sprintf("<%s Chain>", chainID),
-			logging.NewWrappedCore(
-				logging.Info,
-				originalStderr,
-				logging.Colors.ConsoleEncoder(),
-			),
-		),
+		Log:          vm.log,
 		Keystore:     keystoreClient,
 		SharedMemory: sharedMemoryClient,
 		BCLookup:     bcLookupClient,
@@ -652,6 +655,7 @@ func (vm *VMServer) GetAncestors(ctx context.Context, req *vmpb.GetAncestorsRequ
 
 	blocks, err := block.GetAncestors(
 		ctx,
+		vm.log,
 		vm.vm,
 		blkID,
 		maxBlksNum,