store curr cache stat as the baseline of prev cache before the rotation

Signed-off-by: “Jayice” <jzhou@victoriametrics.com>

app/vmalert/config/testdata/rules/rules-replay-good.rules

@@ -18,7 +18,7 @@ groups:
     concurrency: 2
     rules:
       - alert: RequestErrorsToAPI
-        expr: increase(vm_http_request_errors_total{path=~".+"}[5m]) > 0
+        expr: increase(vm_http_request_errors_total[5m]) > 0
         for: 15m
         labels:
           severity: warning

deployment/docker/rules/alerts-cluster.yml

@@ -75,7 +75,7 @@ groups:
             Consider to limit the ingestion rate, decrease retention or scale the disk space if possible."
 
       - alert: RequestErrorsToAPI
-        expr: increase(vm_http_request_errors_total{path=~".+", path!="*"}[5m]) > 0
+        expr: increase(vm_http_request_errors_total[5m]) > 0
         for: 15m
         labels:
           severity: warning
@@ -83,24 +83,8 @@ groups:
         annotations:
           dashboard: "{{ $externalURL }}/d/oS7Bi_0Wz?viewPanel=52&var-instance={{ $labels.instance }}"
           summary: "Too many errors served for {{ $labels.job }} path {{ $labels.path }} (instance {{ $labels.instance }})"
-          description: |
-              Requests to path {{ $labels.path }} are receiving errors.
-              Please verify if clients are sending correct requests.
-
-      # Auth errors and unknown paths should be handled by a different alert
-      # See https://github.com/VictoriaMetrics/VictoriaMetrics/blob/fdd9a221df835daa378ae2e6c9f12e4e3be79c76/lib/httpserver/httpserver.go#L589-L591
-      - alert: RequestErrorsToUnknownPaths
-        expr: sum(increase(vm_http_request_errors_total{path=~"^(\*|)$"}[5m])) by(job, instance, reason) > 0
-        for: 15m
-        labels:
-          severity: warning
-          show_at: dashboard
-        annotations:
-          dashboard: "{{ $externalURL }}/d/oS7Bi_0Wz?viewPanel=52&var-instance={{ $labels.instance }}"
-          summary: "Too many errors served for {{ $labels.job }} with reason {{ $labels.reason }} (instance {{ $labels.instance }})"
-          description: |
-              Requests are failing with reason {{ $labels.reason }}.
-              Please verify if clients are sending correct requests.
+          description: "Requests to path {{ $labels.path }} are receiving errors.
+            Please verify if clients are sending correct requests."
 
       - alert: RPCErrors
         expr: |

deployment/docker/rules/alerts-single-node.yml

@@ -75,7 +75,7 @@ groups:
             Consider to limit the ingestion rate, decrease retention or scale the disk space if possible."
 
       - alert: RequestErrorsToAPI
-        expr: increase(vm_http_request_errors_total{path=~".+"}[5m]) > 0
+        expr: increase(vm_http_request_errors_total[5m]) > 0
         for: 15m
         labels:
           severity: warning

docs/victoriametrics/changelog/CHANGELOG.md

@@ -25,8 +25,7 @@ The sandbox cluster installation runs under the constant load generated by
 See also [LTS releases](https://docs.victoriametrics.com/victoriametrics/lts-releases/).
 
 ## tip
-
-* FEATURE: [alerts](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/rules): update `RequestErrorsToAPI` alerting rule and add `RequestErrorsToUnknownPaths` to [cluster alerts](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/rules/alerts-cluster.yml). The new rule notifies when authentication fails or unknown paths are requested. Previously both cases were treated as errors to the API.
+* BUGFIX: [vmsingle](https://docs.victoriametrics.com/victoriametrics/single-server-victoriametrics/), `vmstorage` and `vmselect` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/): persist the previous working set cache during graceful shutdown when it is likely to contain the active working set. This prevents saving an empty or cold current cache right after split-mode cache rotation, which could otherwise slow down ingestion or queries after restart until the cache warms up again. See [#11299](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/11299).
 
 ## [v1.149.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.149.0)
 

lib/workingsetcache/cache.go

@@ -30,6 +30,12 @@ const (
 	modeWhole     = 2
 )
 
+const (
+	// minCurrCacheSaveMissRate is the minimum miss rate of curr cache
+	// for saving prev instead of curr during split mode.
+	minCurrCacheSaveMissRate = 0.8
+)
+
 // Cache is a cache for working set entries.
 //
 // The cache evicts inactive entries after the given expireDuration.
@@ -41,6 +47,10 @@ type Cache struct {
 	// csHistory holds cache stats history
 	csHistory fastcache.Stats
 
+	// prevStatsAtRotation holds prev cache stats at the moment it became prev from curr.
+	// It is used for calculating prev miss rate since the last cache rotation.
+	prevStatsAtRotation fastcache.Stats
+
 	// mode indicates whether to use only curr and skip prev.
 	//
 	// This flag is set to modeSwitching if curr is filled for more than 50% space.
@@ -145,6 +155,7 @@ func newCacheInternal(curr, prev *fastcache.Cache, mode, maxBytes int, expireDur
 	c.maxBytes = maxBytes
 	c.curr.Store(curr)
 	c.prev.Store(prev)
+	prev.UpdateStats(&c.prevStatsAtRotation)
 	c.stopCh = make(chan struct{})
 	c.mode.Store(uint32(mode))
 	c.runWatchers(expireDuration)
@@ -184,7 +195,7 @@ func (c *Cache) expirationWatcher(expireDuration time.Duration) {
 		prev := c.prev.Load()
 		curr := c.curr.Load()
 		c.updateCacheStatsHistoryBeforeRotationLocked(prev, curr)
-
+		c.storeCurrStatsBeforeRotationLocked(curr)
 		c.prev.Store(curr)
 		prev.Reset()
 		c.curr.Store(prev)
@@ -305,7 +316,7 @@ func (c *Cache) transitIntoWholeModeLocked(maxBytesSize uint64, t *time.Ticker)
 	prev := c.prev.Load()
 	curr := c.curr.Load()
 	c.updateCacheStatsHistoryBeforeRotationLocked(prev, curr)
-
+	c.storeCurrStatsBeforeRotationLocked(curr)
 	c.prev.Store(curr)
 	prev.Reset()
 
@@ -356,6 +367,7 @@ func (c *Cache) transitIntoWholeModeLocked(maxBytesSize uint64, t *time.Ticker)
 	c.updateCacheStatsHistoryBeforeRotationLocked(prev, curr)
 
 	c.prev.Store(newWithAutoCleanup(1024))
+	c.prevStatsAtRotation.Reset()
 	prev.Reset()
 }
 
@@ -363,14 +375,15 @@ func (c *Cache) transitIntoWholeModeLocked(maxBytesSize uint64, t *time.Ticker)
 func (c *Cache) MustSave(filePath string) {
 	startTime := time.Now()
 
-	var cs fastcache.Stats
-	curr := c.curr.Load()
-	curr.UpdateStats(&cs)
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	cacheToSave, cs, cacheName := c.selectCacheToSave()
 
 	concurrency := cgroup.AvailableCPUs()
 
-	logger.Infof("saving cache to %s by using %d concurrent workers", filePath, concurrency)
-	err := curr.SaveToFileConcurrent(filePath, concurrency)
+	logger.Infof("saving %s cache to %s by using %d concurrent workers", cacheName, filePath, concurrency)
+	err := cacheToSave.SaveToFileConcurrent(filePath, concurrency)
 	if err != nil {
 		logger.Panicf("FATAL: cannot save cache to %s: %s", filePath, err)
 	}
@@ -378,6 +391,47 @@ func (c *Cache) MustSave(filePath string) {
 	logger.Infof("cache has been successfully saved to %s in %.3f seconds; entriesCount: %d, sizeBytes: %d", filePath, time.Since(startTime).Seconds(), cs.EntriesCount, cs.BytesSize)
 }
 
+func (c *Cache) selectCacheToSave() (*fastcache.Cache, fastcache.Stats, string) {
+	curr := c.curr.Load()
+
+	var csCurr fastcache.Stats
+	curr.UpdateStats(&csCurr)
+
+	if c.mode.Load() != modeSplit {
+		return curr, csCurr, "curr"
+	}
+
+	prev := c.prev.Load()
+
+	var csPrev fastcache.Stats
+	prev.UpdateStats(&csPrev)
+
+	if csPrev.EntriesCount == 0 || csPrev.GetCalls == 0 {
+		return curr, csCurr, "curr"
+	}
+
+	csPrevAtRotation := &c.prevStatsAtRotation
+	prevMissRateAfterRotation := float64(1)
+	if csPrev.GetCalls > csPrevAtRotation.GetCalls {
+		prevGetCallsAfterRotation := csPrev.GetCalls - csPrevAtRotation.GetCalls
+		prevMissesAfterRotation := uint64(0)
+		if csPrev.Misses > csPrevAtRotation.Misses {
+			prevMissesAfterRotation = csPrev.Misses - csPrevAtRotation.Misses
+		}
+		if prevMissesAfterRotation < prevGetCallsAfterRotation {
+			prevMissRateAfterRotation = float64(prevMissesAfterRotation) / float64(prevGetCallsAfterRotation)
+		}
+	}
+
+	// Prefer saving prev cache when:
+	// 1. 80% requests were missed in curr cache and served by prev cache.
+	// 2. less than 80% requests were missed in prev cache since the last rotation.
+	if csCurr.GetCalls < 10 || (float64(csCurr.Misses)/float64(csCurr.GetCalls) > minCurrCacheSaveMissRate && prevMissRateAfterRotation < minCurrCacheSaveMissRate) {
+		return prev, csPrev, "prev"
+	}
+	return curr, csCurr, "curr"
+}
+
 // Stop stops the cache.
 //
 // The cache cannot be used after the Stop call.
@@ -406,12 +460,18 @@ func (c *Cache) Reset() {
 		// so we have to restore it into original size for split mode
 		c.prev.Store(newWithAutoCleanup(c.maxBytes / 2))
 		c.curr.Store(newWithAutoCleanup(c.maxBytes / 2))
-
+		c.prevStatsAtRotation.Reset()
 		c.mode.Store(modeSplit)
 	}
 
 	prev.Reset()
 	curr.Reset()
+	c.prevStatsAtRotation.Reset()
+}
+
+func (c *Cache) storeCurrStatsBeforeRotationLocked(curr *fastcache.Cache) {
+	c.prevStatsAtRotation.Reset()
+	curr.UpdateStats(&c.prevStatsAtRotation)
 }
 
 // UpdateStats updates fcs with cache stats.

lib/workingsetcache/cache_synctest_test.go

@@ -5,6 +5,7 @@ package workingsetcache
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"testing"
 	"testing/synctest"
 	"time"
@@ -165,6 +166,163 @@ func TestSetGetStatsInSplitMode_cacheLoadedFromEmptyFile(t *testing.T) {
 	})
 }
 
+func TestMustSaveSelectsCacheInSplitMode(t *testing.T) {
+	t.Run("prefers prev cache if curr is rarely visited", func(t *testing.T) {
+		cachePath := filepath.Join(t.TempDir(), "cache")
+		synctest.Test(t, func(t *testing.T) {
+			var (
+				k   = []byte("k")
+				v   = []byte("v")
+				dst []byte
+			)
+
+			c := Load(cachePath, 1024)
+			c.Set(k, v)
+			for range 10 {
+				dst = c.Get(dst[:0], k)
+			}
+
+			// prev and curr were rotated, k is now in prev, curr is empty.
+			time.Sleep(*cacheExpireDuration + time.Minute)
+			synctest.Wait()
+			assertMode(t, c, modeSplit)
+
+			c.MustSave(cachePath)
+			c.Stop()
+
+			c = Load(cachePath, 1024)
+			defer c.Stop()
+			if got := c.Get(dst[:0], k); string(got) != string(v) {
+				t.Fatalf("unexpected value loaded from saved cache; got %q; want %q", got, v)
+			}
+		})
+	})
+
+	t.Run("prefers prev cache when prev is still useful", func(t *testing.T) {
+		cachePath := filepath.Join(t.TempDir(), "cache")
+		synctest.Test(t, func(t *testing.T) {
+			const keysCount = 10
+			var (
+				v   = []byte("v")
+				dst []byte
+			)
+
+			c := Load(cachePath, 1024)
+			for i := range keysCount {
+				c.Set([]byte(fmt.Sprintf("prev_%d", i)), v)
+			}
+
+			// prev and curr were rotated, prev_0-prev_9 are now in prev, curr is empty.
+			time.Sleep(*cacheExpireDuration + time.Minute)
+			synctest.Wait()
+			assertMode(t, c, modeSplit)
+
+			// all get calls are missed in curr cache, but can be served by prev cache.
+			for i := range keysCount {
+				dst = c.Get(dst[:0], []byte(fmt.Sprintf("prev_%d", i)))
+				if string(dst) != string(v) {
+					t.Fatalf("unexpected value loaded from prev cache for key %q; got %q; want %q", fmt.Sprintf("prev_%d", i), dst, v)
+				}
+			}
+
+			c.MustSave(cachePath)
+			c.Stop()
+
+			c = Load(cachePath, 1024)
+			defer c.Stop()
+			for i := range keysCount {
+				key := []byte(fmt.Sprintf("prev_%d", i))
+				if got := c.Get(dst[:0], key); string(got) != string(v) {
+					t.Fatalf("unexpected value loaded from saved cache for key %q; got %q; want %q", key, got, v)
+				}
+			}
+		})
+	})
+
+	t.Run("prefers curr cache when prev is cold", func(t *testing.T) {
+		cachePath := filepath.Join(t.TempDir(), "cache")
+		synctest.Test(t, func(t *testing.T) {
+			const keysCount = 10
+			var (
+				v   = []byte("v")
+				dst []byte
+			)
+
+			c := Load(cachePath, 1024)
+			for i := range keysCount {
+				c.Set([]byte(fmt.Sprintf("prev_%d", i)), v)
+			}
+
+			// prev and curr were rotated, prev_0-prev_9 are now in prev, curr is empty.
+			time.Sleep(*cacheExpireDuration + time.Minute)
+			synctest.Wait()
+			assertMode(t, c, modeSplit)
+
+			// all get calls are missed in both curr and prev cache.
+			for i := range keysCount {
+				newKey := []byte(fmt.Sprintf("new_%d", i))
+				dst = c.Get(dst[:0], newKey)
+			}
+
+			c.MustSave(cachePath)
+			c.Stop()
+
+			c = Load(cachePath, 1024)
+			defer c.Stop()
+			for i := range keysCount {
+				prevKey := []byte(fmt.Sprintf("prev_%d", i))
+				if got := c.Get(dst[:0], prevKey); len(got) != 0 {
+					t.Fatalf("unexpected prev value loaded from saved cache for key %q; got %q; want an empty value", prevKey, got)
+				}
+			}
+		})
+	})
+
+	t.Run("prefers curr cache when prev hits rate is low after rotation", func(t *testing.T) {
+		cachePath := filepath.Join(t.TempDir(), "cache")
+		synctest.Test(t, func(t *testing.T) {
+			const keysCount = 10
+			var (
+				prevKey = []byte("prev")
+				currKey = []byte("curr")
+				v       = []byte("v")
+				dst     []byte
+			)
+
+			c := Load(cachePath, 1024)
+			c.Set(prevKey, v)
+			// the curr cache hit all the requests.
+			for range keysCount {
+				dst = c.Get(dst[:0], prevKey)
+			}
+
+			// prev and curr were rotated, prevKey is now in prev, whose cache hit ratio is 100%.
+			time.Sleep(*cacheExpireDuration + time.Minute)
+			synctest.Wait()
+			assertMode(t, c, modeSplit)
+
+			c.Set(currKey, v)
+			// the prev cache miss all the requests after the rotation
+			for i := range keysCount {
+				newKey := []byte(fmt.Sprintf("new_%d", i))
+				dst = c.Get(dst[:0], newKey)
+			}
+
+			c.MustSave(cachePath)
+			c.Stop()
+
+			c = Load(cachePath, 1024)
+			defer c.Stop()
+			if got := c.Get(dst[:0], currKey); string(got) != string(v) {
+				t.Fatalf("unexpected value loaded from saved cache for key %q; got %q; want %q", currKey, got, v)
+			}
+			if got := c.Get(dst[:0], prevKey); len(got) != 0 {
+				t.Fatalf("unexpected prev value loaded from saved cache for key %q; got %q; want an empty value", prevKey, got)
+			}
+		})
+	})
+}
+
 func testSetGetStatsInSplitMode(t *testing.T, c *Cache) {
 	var (
 		k1, v1  = []byte("k1"), []byte("v1")