#3046 persistent failed item status and #2065 failed items reporting error once (#3047)

* added logs for debug

* fixes

* removed logs

* using cache item

* implemented multiple tests

* fixed some unit tests

* implemented test for skipping

* added multiple tests together

* added mark failed

* fix on tests

* better test implementation + concurrent

* fix: fixes on concurrent tests

* removed parallel and 1 unit test

DOCS: by default the command go test runs in parallel tests for different packages, and default is the number of CPUs available (see go help build)

* fixes on go routine

* increasing parallelism of once.Do

* bumping go to 1.19 for atomic types support

* removing redundant check + fixing test concurrency on create

Co-authored-by: Mzack9999 <mzack9999@protonmail.com>
Co-authored-by: mzack <marco.rivoli.nvh@gmail.com>

v2/pkg/protocols/common/hosterrorscache/hosterrorscache_test.go

@@ -2,30 +2,122 @@ package hosterrorscache
 
 import (
 	"fmt"
+	"sync"
+	"sync/atomic"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 )
 
-func TestCacheCheckMarkFailed(t *testing.T) {
+func TestCacheCheck(t *testing.T) {
 	cache := New(3, DefaultMaxHostsCount)
 
-	cache.MarkFailed("http://example.com:80", fmt.Errorf("no address found for host"))
-	if value, err := cache.failedTargets.Get("http://example.com:80"); err == nil && value != nil {
-		require.Equal(t, 1, value, "could not get correct number of marked failed hosts")
-	}
-	cache.MarkFailed("example.com:80", fmt.Errorf("Client.Timeout exceeded while awaiting headers"))
-	if value, err := cache.failedTargets.Get("example.com:80"); err == nil && value != nil {
-		require.Equal(t, 2, value, "could not get correct number of marked failed hosts")
-	}
-	cache.MarkFailed("example.com", fmt.Errorf("could not resolve host"))
-	if value, err := cache.failedTargets.Get("example.com"); err == nil && value != nil {
-		require.Equal(t, 1, value, "could not get correct number of marked failed hosts")
-	}
-	for i := 0; i < 3; i++ {
+	for i := 0; i < 100; i++ {
 		cache.MarkFailed("test", fmt.Errorf("could not resolve host"))
+		got := cache.Check("test")
+		if i < 2 {
+			// till 3 the host is not flagged to skip
+			require.False(t, got)
+		} else {
+			// above 3 it must remain flagged to skip
+			require.True(t, got)
+		}
 	}
 
 	value := cache.Check("test")
 	require.Equal(t, true, value, "could not get checked value")
 }
+
+func TestCacheItemDo(t *testing.T) {
+	var (
+		count int
+		item  cacheItem
+	)
+
+	wg := sync.WaitGroup{}
+	for i := 0; i < 100; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			item.Do(func() {
+				count++
+			})
+		}()
+	}
+	wg.Wait()
+
+	// ensures the increment happened only once regardless of the multiple call
+	require.Equal(t, count, 1)
+}
+
+func TestCacheMarkFailed(t *testing.T) {
+	cache := New(3, DefaultMaxHostsCount)
+
+	tests := []struct {
+		host     string
+		expected int
+	}{
+		{"http://example.com:80", 1},
+		{"example.com:80", 2},
+		{"example.com", 1},
+	}
+
+	for _, test := range tests {
+		normalizedCacheValue := cache.normalizeCacheValue(test.host)
+		cache.MarkFailed(test.host, fmt.Errorf("no address found for host"))
+		failedTarget, err := cache.failedTargets.Get(normalizedCacheValue)
+		require.Nil(t, err)
+		require.NotNil(t, failedTarget)
+
+		value, ok := failedTarget.(*cacheItem)
+		require.True(t, ok)
+		require.EqualValues(t, test.expected, value.errors.Load())
+	}
+}
+
+func TestCacheMarkFailedConcurrent(t *testing.T) {
+	cache := New(3, DefaultMaxHostsCount)
+
+	tests := []struct {
+		host     string
+		expected int32
+	}{
+		{"http://example.com:80", 200},
+		{"example.com:80", 200},
+		{"example.com", 100},
+	}
+
+	// the cache is not atomic during items creation, so we pre-create them with counter to zero
+	for _, test := range tests {
+		normalizedValue := cache.normalizeCacheValue(test.host)
+		newItem := &cacheItem{errors: atomic.Int32{}}
+		newItem.errors.Store(0)
+		_ = cache.failedTargets.Set(normalizedValue, newItem)
+	}
+
+	wg := sync.WaitGroup{}
+	for _, test := range tests {
+		currentTest := test
+		for i := 0; i < 100; i++ {
+			wg.Add(1)
+			go func() {
+				defer wg.Done()
+				cache.MarkFailed(currentTest.host, fmt.Errorf("could not resolve host"))
+			}()
+		}
+	}
+	wg.Wait()
+
+	for _, test := range tests {
+		require.True(t, cache.Check(test.host))
+
+		normalizedCacheValue := cache.normalizeCacheValue(test.host)
+		failedTarget, err := cache.failedTargets.Get(normalizedCacheValue)
+		require.Nil(t, err)
+		require.NotNil(t, failedTarget)
+
+		value, ok := failedTarget.(*cacheItem)
+		require.True(t, ok)
+		require.EqualValues(t, test.expected, value.errors.Load())
+	}
+}