From 4a359d5f677aa0086960725a0e35a8e9d15e4b50 Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
Date: Wed, 27 Mar 2024 10:51:03 +0200
Subject: [PATCH] lib/storage: follow-up for
 76f00cea6b9bf02a262275a112040c2680ed472c

Store the deadline when the metricID entries must be deleted from indexdb
if metricID->metricName entry isn't found after the deadline. This should
make the code more clear comparing the the previous version, where the timestamp
of the first metricID->metricName lookup miss was stored in missingMetricIDs.

Remove the misleading comment about the importance of the order for creating entries
in the inverted index when registering new time series. The order doesn't matter,
since any subset of the created entries can become visible for search
before any other subset after registering in indexdb.

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5948
Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5959
---
 lib/storage/index_db.go | 11 ++++-------
 lib/storage/storage.go  |  8 +++++---
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/lib/storage/index_db.go b/lib/storage/index_db.go
index 26728b039..bf30836e0 100644
--- a/lib/storage/index_db.go
+++ b/lib/storage/index_db.go
@@ -506,9 +506,6 @@ func generateTSID(dst *TSID, mn *MetricName) {
 }
 
 func (is *indexSearch) createGlobalIndexes(tsid *TSID, mn *MetricName) {
-	// The order of index items is important.
-	// It guarantees index consistency.
-
 	ii := getIndexItems()
 	defer putIndexItems(ii)
 
@@ -1504,17 +1501,17 @@ func (db *indexDB) searchMetricNameWithCache(dst []byte, metricID uint64) ([]byt
 		db.s.missingMetricIDs = nil
 		db.s.missingMetricIDsResetDeadline = ct + 2*60
 	}
-	timestamp, ok := db.s.missingMetricIDs[metricID]
+	deleteDeadline, ok := db.s.missingMetricIDs[metricID]
 	if !ok {
 		if db.s.missingMetricIDs == nil {
 			db.s.missingMetricIDs = make(map[uint64]uint64)
 		}
-		db.s.missingMetricIDs[metricID] = ct
-		timestamp = ct
+		deleteDeadline = ct + 60
+		db.s.missingMetricIDs[metricID] = deleteDeadline
 	}
 	db.s.missingMetricIDsLock.Unlock()
 
-	if ct > timestamp+60 {
+	if ct > deleteDeadline {
 		// Cannot find the MetricName for the given metricID for the last 60 seconds.
 		// It is likely the indexDB contains incomplete set of metricID -> metricName entries
 		// after unclean shutdown or after restoring from a snapshot.
diff --git a/lib/storage/storage.go b/lib/storage/storage.go
index 8be641051..be09f9db5 100644
--- a/lib/storage/storage.go
+++ b/lib/storage/storage.go
@@ -147,9 +147,11 @@ type Storage struct {
 	deletedMetricIDs           atomic.Pointer[uint64set.Set]
 	deletedMetricIDsUpdateLock sync.Mutex
 
-	// missingMetricIDs maps metricID to the timestamp of first unsuccessful lookup
-	// of metricName by the given metricID.
+	// missingMetricIDs maps metricID to the deadline in unix timestamp seconds
+	// after which all the indexdb entries for the given metricID
+	// must be deleted if metricName isn't found by the given metricID.
 	// This is used inside searchMetricNameWithCache() for detecting permanently missing metricID->metricName entries.
+	// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5959
 	missingMetricIDsLock          sync.Mutex
 	missingMetricIDs              map[uint64]uint64
 	missingMetricIDsResetDeadline uint64
@@ -1151,7 +1153,7 @@ func (s *Storage) SearchMetricNames(qt *querytracer.Tracer, tfss []*TagFilters,
 		metricName, ok = idb.searchMetricNameWithCache(metricName[:0], metricID)
 		if !ok {
 			// Skip missing metricName for metricID.
-			// It should be automatically fixed. See indexDB.searchMetricName for details.
+			// It should be automatically fixed. See indexDB.searchMetricNameWithCache for details.
 			continue
 		}
 		if _, ok := metricNamesSeen[string(metricName)]; ok {