From 9dae877aeb5b54fb2a40ec4998140da3c273cf43 Mon Sep 17 00:00:00 2001
From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Date: Tue, 28 Jul 2026 13:06:55 -0700
Subject: [PATCH] Fix DeletePartialMatch deleting unrelated metrics on hash
 collision

Motivation:
Issue #1810 reports metrics not being freed as expected after calling
DeletePartialMatch(). While investigating, a distinct, reproducible bug
was found in metricMap.deleteByLabels (the function backing
MetricVec.DeletePartialMatch): when two or more differently-labeled
metrics land in the same hash bucket (a collision), and only one of
them matches the partial-label deletion criteria, the old code deleted
the *entire* bucket via `delete(m.metrics, h)` instead of only the
matching entry - silently discarding unrelated metrics that were never
meant to be deleted. Delete() and DeleteLabelValues() already handled
this correctly elsewhere in the same file (splicing out only the
matching slice element); DeletePartialMatch did not.

Additionally, the old single-match code path did not stop after
removing one bucket, and any collision-safe removal needs to clear the
vacated slice slot so it does not keep referencing a Metric object
through the backing array's spare capacity, which would prevent that
Metric from being garbage collected.

This bug is a plausible contributor to reports like #1810, but the
original reporter's case was never profiled, so it is not confirmed as
the root cause. Hash collisions on the label-value hash are rare in
practice, so this alone may not fully explain a leak; regardless, the
incorrect bucket-wide deletion is a real correctness bug worth fixing
on its own.

Approach:
Rewrite deleteByLabels to filter each hash bucket's slice in place,
removing only the entries that actually match the partial-label
criteria (there can be more than one match per bucket), and explicitly
zero the vacated tail slots of the reused backing array so deleted
Metric objects don't stay reachable. Removed findMetricWithPartialLabels,
which became unused once its only call site was replaced.

Validation:
Added TestDeletePartialMatchWithCollisions, following the existing
TestDeleteWithCollisions / TestDeleteLabelValuesWithCollisions pattern:
forces all label combinations into a single hash bucket via a
stubbed-out hashAdd/hashAddByte, then reuses the existing
testDeletePartialMatch test body. Confirmed this test fails against the
pre-fix code (unrelated collided metrics vanish) and passes after the
fix.

  go build ./...
  go test ./prometheus/... -run 'TestDelete|TestMetricVec|TestCurryVec' -race -v

All of the above passed. (The full `go test ./prometheus/...` has two
pre-existing, unrelated failures on this checkout's Go toolchain, caused
by a godebug runtime metric mismatch in go_collector_latest_test.go;
they reproduce identically on main without this change and are
unaffected by it.)

Fixes #1810

Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
---
 prometheus/vec.go      | 37 ++++++++++++++++++-------------------
 prometheus/vec_test.go | 13 +++++++++++++
 2 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/prometheus/vec.go b/prometheus/vec.go
index 121d2a963..9749dceb2 100644
--- a/prometheus/vec.go
+++ b/prometheus/vec.go
@@ -416,31 +416,30 @@ func (m *metricMap) deleteByLabels(labels Labels, curry []curriedLabelValue) int
 	var numDeleted int
 
 	for h, metrics := range m.metrics {
-		i := findMetricWithPartialLabels(m.desc, metrics, labels, curry)
-		if i >= len(metrics) {
-			// Didn't find matching labels in this metric slice.
-			continue
+		remaining := metrics[:0]
+		for _, metric := range metrics {
+			if matchPartialLabels(m.desc, metric.values, labels, curry) {
+				numDeleted++
+				continue
+			}
+			remaining = append(remaining, metric)
+		}
+		// Clear the tail of the backing array so deleted metrics don't
+		// stay reachable (and hence unable to be garbage collected)
+		// through the array's spare capacity.
+		for i := len(remaining); i < len(metrics); i++ {
+			metrics[i] = metricWithLabelValues{}
+		}
+		if len(remaining) == 0 {
+			delete(m.metrics, h)
+		} else {
+			m.metrics[h] = remaining
 		}
-		delete(m.metrics, h)
-		numDeleted++
 	}
 
 	return numDeleted
 }
 
-// findMetricWithPartialLabel returns the index of the matching metric or
-// len(metrics) if not found.
-func findMetricWithPartialLabels(
-	desc *Desc, metrics []metricWithLabelValues, labels Labels, curry []curriedLabelValue,
-) int {
-	for i, metric := range metrics {
-		if matchPartialLabels(desc, metric.values, labels, curry) {
-			return i
-		}
-	}
-	return len(metrics)
-}
-
 // indexOf searches the given slice of strings for the target string and returns
 // the index or len(items) as well as a boolean whether the search succeeded.
 func indexOf(target string, items []string) (int, bool) {
diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go
index 03223f2f6..1c8b86df8 100644
--- a/prometheus/vec_test.go
+++ b/prometheus/vec_test.go
@@ -166,6 +166,19 @@ func TestDeletePartialMatch(t *testing.T) {
 	testDeletePartialMatch(t, vec)
 }
 
+func TestDeletePartialMatchWithCollisions(t *testing.T) {
+	vec := NewGaugeVec(
+		GaugeOpts{
+			Name: "test",
+			Help: "helpless",
+		},
+		[]string{"l1", "l2", "l3"},
+	)
+	vec.hashAdd = func(h uint64, s string) uint64 { return 1 }
+	vec.hashAddByte = func(h uint64, b byte) uint64 { return 1 }
+	testDeletePartialMatch(t, vec)
+}
+
 func TestDeletePartialMatchWithConstraints(t *testing.T) {
 	vec := V2.NewGaugeVec(GaugeVecOpts{
 		GaugeOpts{