From eb5288f4a5f00b3389a38073bd49b4486678749a Mon Sep 17 00:00:00 2001
From: Alex Vanin <a.vanin@yadro.com>
Date: Thu, 28 Sep 2023 13:22:30 +0300
Subject: [PATCH] [#172] pool: Do more retries on unexpected tree service
 responses

1. Try its best by looking for nodes during 'GetNodeByPath'
2. Retry on 'tree not found' and other not found errors

Signed-off-by: Alex Vanin <a.vanin@yadro.com>
---
 pool/tree/pool.go      | 16 ++++++++++++----
 pool/tree/pool_test.go | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/pool/tree/pool.go b/pool/tree/pool.go
index a9adb01..b3739c9 100644
--- a/pool/tree/pool.go
+++ b/pool/tree/pool.go
@@ -32,6 +32,9 @@ var (
 
 	// ErrNodeAccessDenied is returned from Tree service in case of access denied error.
 	ErrNodeAccessDenied = errors.New("access denied")
+
+	// errNodeEmpty is used to trigger retry when 'GetNodeByPath' return empty result.
+	errNodeEmptyResult = errors.New("empty result")
 )
 
 // client represents virtual connection to the single FrostFS tree service from which Pool is formed.
@@ -296,8 +299,15 @@ func (p *Pool) GetNodes(ctx context.Context, prm GetNodesParams) ([]*grpcService
 	var resp *grpcService.GetNodeByPathResponse
 	if err := p.requestWithRetry(func(client grpcService.TreeServiceClient) (inErr error) {
 		resp, inErr = client.GetNodeByPath(ctx, request)
+		// Pool wants to do retry 'GetNodeByPath' request if result is empty.
+		// Empty result is expected due to delayed tree service sync.
+		// Return an error there to trigger retry and ignore it after,
+		// to keep compatibility with 'GetNodeByPath' implementation.
+		if inErr == nil && len(resp.Body.Nodes) == 0 {
+			return errNodeEmptyResult
+		}
 		return handleError("failed to get node by path", inErr)
-	}); err != nil {
+	}); err != nil && !errors.Is(err, errNodeEmptyResult) {
 		return nil, err
 	}
 
@@ -749,7 +759,5 @@ func (p *Pool) requestWithRetry(fn func(client grpcService.TreeServiceClient) er
 }
 
 func shouldTryAgain(err error) bool {
-	return !(err == nil ||
-		errors.Is(err, ErrNodeNotFound) ||
-		errors.Is(err, ErrNodeAccessDenied))
+	return !(err == nil || errors.Is(err, ErrNodeAccessDenied))
 }
diff --git a/pool/tree/pool_test.go b/pool/tree/pool_test.go
index 1c73205..2c616a7 100644
--- a/pool/tree/pool_test.go
+++ b/pool/tree/pool_test.go
@@ -156,6 +156,43 @@ func TestRetry(t *testing.T) {
 		require.NoError(t, err)
 		checkIndicesAndReset(t, p, 0, 0)
 	})
+
+	t.Run("error empty result", func(t *testing.T) {
+		errNodes, index := 2, 0
+		err := p.requestWithRetry(func(client grpcService.TreeServiceClient) error {
+			if index < errNodes {
+				index++
+				return errNodeEmptyResult
+			}
+			return nil
+		})
+		require.NoError(t, err)
+		checkIndicesAndReset(t, p, 0, errNodes)
+	})
+
+	t.Run("error not found", func(t *testing.T) {
+		errNodes, index := 2, 0
+		err := p.requestWithRetry(func(client grpcService.TreeServiceClient) error {
+			if index < errNodes {
+				index++
+				return ErrNodeNotFound
+			}
+			return nil
+		})
+		require.NoError(t, err)
+		checkIndicesAndReset(t, p, 0, errNodes)
+	})
+
+	t.Run("error access denied", func(t *testing.T) {
+		var index int
+		err := p.requestWithRetry(func(client grpcService.TreeServiceClient) error {
+			index++
+			return ErrNodeAccessDenied
+		})
+		require.ErrorIs(t, err, ErrNodeAccessDenied)
+		require.Equal(t, 1, index)
+		checkIndicesAndReset(t, p, 0, 0)
+	})
 }
 
 func TestRebalance(t *testing.T) {