track oldest delete request age since their cancellation period is over (#2806)

* track oldest delete request age since their cancellation period is over

Signed-off-by: Sandeep Sukhani <[email protected]>

* update metric for oldest pending request and number of pending requests to 0 sooner

Signed-off-by: Sandeep Sukhani <[email protected]>

* changes suggested from PR review

Signed-off-by: Sandeep Sukhani <[email protected]>

* update changelog

Signed-off-by: Sandeep Sukhani <[email protected]>

* minor nits suggested in PR review

Signed-off-by: Sandeep Sukhani <[email protected]>

* Fixed flaky test

Signed-off-by: Marco Pracucci <[email protected]>

Co-authored-by: Marco Pracucci <[email protected]>

Author: sandeepsukhani

CHANGELOG.md

@@ -8,6 +8,7 @@
   * `cortex_bucket_stores_gate_queries_in_flight`
   * `cortex_bucket_stores_gate_duration_seconds`
 * [CHANGE] Metric `cortex_ingester_flush_reasons` has been renamed to `cortex_ingester_series_flushed_total`, and is now incremented during flush, not when series is enqueued for flushing. #2802
+* [CHANGE] Experimental Delete Series: Metric `cortex_purger_oldest_pending_delete_request_age_seconds` would track age of delete requests since they are over their cancellation period instead of their creation time. #2806
 * [FEATURE] Introduced `ruler.for-outage-tolerance`, Max time to tolerate outage for restoring "for" state of alert. #2783
 * [FEATURE] Introduced `ruler.for-grace-period`, Minimum duration between alert and restored "for" state. This is maintained only for alerts with configured "for" time greater than grace period. #2783
 * [FEATURE] Introduced `ruler.resend-delay`, Minimum amount of time to wait before resending an alert to Alertmanager. #2783

pkg/chunk/purger/purger.go

@@ -66,12 +66,12 @@ func newPurgerMetrics(r prometheus.Registerer) *purgerMetrics {
 	m.oldestPendingDeleteRequestAgeSeconds = promauto.With(r).NewGauge(prometheus.GaugeOpts{
 		Namespace: "cortex",
 		Name:      "purger_oldest_pending_delete_request_age_seconds",
-		Help:      "Age of oldest pending delete request in seconds",
+		Help:      "Age of oldest pending delete request in seconds, since they are over their cancellation period",
 	})
 	m.pendingDeleteRequestsCount = promauto.With(r).NewGauge(prometheus.GaugeOpts{
 		Namespace: "cortex",
 		Name:      "purger_pending_delete_requests_count",
-		Help:      "Count of requests which are in process or are ready to be processed",
+		Help:      "Count of delete requests which are over their cancellation period and have not finished processing yet",
 	})
 
 	return &m
@@ -248,6 +248,10 @@ func (p *Purger) workerJobCleanup(job workerJob) {
 			default:
 				// already sent
 			}
+		} else if len(p.usersWithPendingRequests) == 0 {
+			// there are no pending requests from any of the users, set the oldest pending request and number of pending requests to 0
+			p.metrics.oldestPendingDeleteRequestAgeSeconds.Set(0)
+			p.metrics.pendingDeleteRequestsCount.Set(0)
 		}
 	} else {
 		p.pendingPlansCountMtx.Unlock()
@@ -409,7 +413,7 @@ func (p *Purger) pullDeleteRequestsToPlanDeletes() error {
 	p.inProcessRequestIDsMtx.RUnlock()
 
 	now := model.Now()
-	oldestPendingRequestCreatedAt := now
+	oldestPendingRequestCreatedAt := model.Time(0)
 
 	// requests which are still being processed are also considered pending
 	if pendingDeleteRequestsCount != 0 {
@@ -426,7 +430,7 @@ func (p *Purger) pullDeleteRequestsToPlanDeletes() error {
 		}
 
 		pendingDeleteRequestsCount++
-		if deleteRequest.CreatedAt.Before(oldestPendingRequestCreatedAt) {
+		if oldestPendingRequestCreatedAt == 0 || deleteRequest.CreatedAt.Before(oldestPendingRequestCreatedAt) {
 			oldestPendingRequestCreatedAt = deleteRequest.CreatedAt
 		}
 
@@ -473,7 +477,12 @@ func (p *Purger) pullDeleteRequestsToPlanDeletes() error {
 		p.executePlansChan <- req
 	}
 
-	p.metrics.oldestPendingDeleteRequestAgeSeconds.Set(float64(now.Sub(oldestPendingRequestCreatedAt) / time.Second))
+	// track age of oldest delete request since they are over their cancellation period
+	oldestPendingRequestAge := time.Duration(0)
+	if oldestPendingRequestCreatedAt != 0 {
+		oldestPendingRequestAge = now.Sub(oldestPendingRequestCreatedAt.Add(p.cfg.DeleteRequestCancelPeriod))
+	}
+	p.metrics.oldestPendingDeleteRequestAgeSeconds.Set(float64(oldestPendingRequestAge / time.Second))
 	p.metrics.pendingDeleteRequestsCount.Set(float64(pendingDeleteRequestsCount))
 
 	return nil

pkg/chunk/purger/purger_test.go

@@ -410,8 +410,8 @@ func TestPurger_Metrics(t *testing.T) {
 	// load new delete requests for processing
 	require.NoError(t, purger.pullDeleteRequestsToPlanDeletes())
 
-	// there must be 2 pending delete requests, oldest being 3 days old
-	require.InDelta(t, float64(3*86400), testutil.ToFloat64(purger.metrics.oldestPendingDeleteRequestAgeSeconds), 1)
+	// there must be 2 pending delete requests, oldest being 2 days old since its cancellation time is over
+	require.InDelta(t, float64(2*86400), testutil.ToFloat64(purger.metrics.oldestPendingDeleteRequestAgeSeconds), 1)
 	require.Equal(t, float64(2), testutil.ToFloat64(purger.metrics.pendingDeleteRequestsCount))
 
 	// start loop to process requests
@@ -429,9 +429,6 @@ func TestPurger_Metrics(t *testing.T) {
 		return testutil.ToFloat64(purger.metrics.deleteRequestsProcessedTotal)
 	})
 
-	// load new delete requests for processing which should update the metrics
-	require.NoError(t, purger.pullDeleteRequestsToPlanDeletes())
-
 	// there must be 0 pending delete requests so the age for oldest pending must be 0
 	require.InDelta(t, float64(0), testutil.ToFloat64(purger.metrics.oldestPendingDeleteRequestAgeSeconds), 1)
 	require.Equal(t, float64(0), testutil.ToFloat64(purger.metrics.pendingDeleteRequestsCount))

pkg/testexporter/correctness/runner_test.go

@@ -27,7 +27,6 @@ func TestMinQueryTime(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		//require.Equal(t, tt.expected, tc.MinQueryTime())
-		assert.WithinDuration(t, tt.expected, calculateMinQueryTime(tt.durationQuerySince, tt.timeQueryStart), 5*time.Millisecond)
+		assert.WithinDuration(t, tt.expected, calculateMinQueryTime(tt.durationQuerySince, tt.timeQueryStart), 50*time.Millisecond)
 	}
 }