From fc4551b84341ab45cf856b88713c1dbfa01afad7 Mon Sep 17 00:00:00 2001
From: Alex Vanin <a.vanin@yadro.com>
Date: Tue, 10 Oct 2023 11:25:57 +0300
Subject: [PATCH] [#172] pool: Use priority of errors in tree pool

When retry happens, use priority map to decide
which error to return. Consider network errors
less desirable than business logic errors.

Signed-off-by: Alex Vanin <a.vanin@yadro.com>
---
 pool/tree/client.go |  8 +++++++-
 pool/tree/pool.go   | 36 +++++++++++++++++++++++++++++++++---
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/pool/tree/client.go b/pool/tree/client.go
index 326a8cc..d6dd57c 100644
--- a/pool/tree/client.go
+++ b/pool/tree/client.go
@@ -3,6 +3,7 @@ package tree
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"sync"
 
@@ -22,6 +23,11 @@ type treeClient struct {
 	healthy bool
 }
 
+var (
+	// ErrUnhealthyEndpoint is returned when client in the pool considered unavailable.
+	ErrUnhealthyEndpoint = errors.New("unhealthy endpoint")
+)
+
 // newTreeClient creates new tree client with auto dial.
 func newTreeClient(addr string, opts ...grpc.DialOption) *treeClient {
 	return &treeClient{
@@ -102,7 +108,7 @@ func (c *treeClient) serviceClient() (grpcService.TreeServiceClient, error) {
 	defer c.mu.RUnlock()
 
 	if c.conn == nil || !c.healthy {
-		return nil, fmt.Errorf("unhealthy endpoint: '%s'", c.address)
+		return nil, fmt.Errorf("%w: '%s'", ErrUnhealthyEndpoint, c.address)
 	}
 
 	return c.service, nil
diff --git a/pool/tree/pool.go b/pool/tree/pool.go
index b3739c9..fc360d2 100644
--- a/pool/tree/pool.go
+++ b/pool/tree/pool.go
@@ -730,8 +730,8 @@ func (p *Pool) setStartIndices(i, j int) {
 
 func (p *Pool) requestWithRetry(fn func(client grpcService.TreeServiceClient) error) error {
 	var (
-		err error
-		cl  grpcService.TreeServiceClient
+		err, finErr error
+		cl          grpcService.TreeServiceClient
 	)
 
 	startI, startJ := p.getStartIndices()
@@ -750,14 +750,44 @@ func (p *Pool) requestWithRetry(fn func(client grpcService.TreeServiceClient) er
 				}
 				return err
 			}
+			finErr = finalError(finErr, err)
 			p.log(zap.DebugLevel, "tree request error", zap.String("address", p.innerPools[indexI].clients[indexJ].endpoint()), zap.Error(err))
 		}
 		startJ = 0
 	}
 
-	return err
+	return finErr
 }
 
 func shouldTryAgain(err error) bool {
 	return !(err == nil || errors.Is(err, ErrNodeAccessDenied))
 }
+
+func prioErr(err error) int {
+	switch {
+	case err == nil:
+		return -1
+	case errors.Is(err, ErrNodeAccessDenied):
+		return 100
+	case errors.Is(err, ErrNodeNotFound) ||
+		errors.Is(err, errNodeEmptyResult):
+		return 200
+	case errors.Is(err, ErrUnhealthyEndpoint):
+		return 300
+	default:
+		return 500
+	}
+}
+
+func finalError(current, candidate error) error {
+	if current == nil || candidate == nil {
+		return candidate
+	}
+
+	// lower priority error is more desirable
+	if prioErr(candidate) < prioErr(current) {
+		return candidate
+	}
+
+	return current
+}