From 181521e075d0cf36a446d91296dd7459c4e1da27 Mon Sep 17 00:00:00 2001
From: Yuri Nikolic <durica.nikolic@grafana.com>
Date: Mon, 7 Sep 2026 23:41:05 +0200
Subject: [PATCH 1/4] Ingester: re-validate ownership before evicting non-owned
 series

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
---
 CHANGELOG.md                                  |   3 +
 pkg/ingester/ingester_compaction.go           |  17 +++
 .../ingester_early_compaction_test.go         | 100 ++++++++++++++++++
 3 files changed, 120 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 94bcb79992b..91cd91f0694 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -251,6 +251,9 @@
 * [ENHANCEMENT] Make range vector splitting configurable per query path. #15706
 * [ENHANCEMENT] Add `newMimirtoolBlocksJob` and subcommand-specific helpers to run `mimirtool blocks` as Kubernetes Jobs. #15757
 * [BUGFIX] Continuous-test: Include `._config.commonConfig` in arguments passed to continuous-test. #15988
+* [BUGFIX] Add missing `-querier.mimir-query-engine.range-vector-splitting.memcached.addresses` to `multi_zone_config_validation_excluded_args`. #16237
+* [BUGFIX] Ingester: Fix a race in early compaction of non-owned series (`-ingester.early-compaction-non-owned-series-enabled`) where a series that became owned again after being queued for eviction, but before the owned-series service reconciled the queue, could still be evicted once its grace period elapsed — silently dropping any samples written to it after ownership was restored. `compactBlocksDueToNonOwnedSeries` now re-validates ownership immediately before evicting queued series. #XXXXX
+
 
 ### Documentation
 
diff --git a/pkg/ingester/ingester_compaction.go b/pkg/ingester/ingester_compaction.go
index e554e4380f6..6cd62211bd4 100644
--- a/pkg/ingester/ingester_compaction.go
+++ b/pkg/ingester/ingester_compaction.go
@@ -630,6 +630,23 @@ func (i *Ingester) compactBlocksDueToNonOwnedSeries(ctx context.Context, jitter
 			continue
 		}
 
+		// Ownership can flip back to this ingester after a series was queued as
+		// non-owned, but before the owned-series service's own ticker notices and
+		// reconciles pendingNonOwnedRefs — this loop runs on a separate schedule
+		// with no guarantee it runs after that reconciliation. Re-running
+		// computeOwnedSeries() here catches that: series that are owned again get
+		// removed from pendingNonOwnedRefs before takePendingNonOwnedRefs can evict
+		// them below. We reuse the ranges already cached on this userTSDB rather
+		// than asking the ring for fresh ones — that's the owned-series service's
+		// job — so this is a no-op when ownership hasn't actually changed.
+		db.pendingNonOwnedRefsMtx.Lock()
+		hasPendingNonOwnedRefs := len(db.pendingNonOwnedRefs) > 0
+		db.pendingNonOwnedRefsMtx.Unlock()
+
+		if hasPendingNonOwnedRefs {
+			db.recomputeOwnedSeries(db.ownedSeriesState().shardSize, "early compaction pre-eviction re-check", i.logger)
+		}
+
 		now := time.Now()
 
 		// Fast path: threshold gate is satisfied and min grace period has elapsed.
diff --git a/pkg/ingester/ingester_early_compaction_test.go b/pkg/ingester/ingester_early_compaction_test.go
index 906bdb1c14d..6e99ae1a419 100644
--- a/pkg/ingester/ingester_early_compaction_test.go
+++ b/pkg/ingester/ingester_early_compaction_test.go
@@ -1697,6 +1697,106 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_StaleRefsAfterPriorEviction(t
 	require.Equal(t, uint64(1), db.Head().NumSeries(), "head should still contain only the owned series")
 }
 
+// TestIngester_compactBlocksDueToNonOwnedSeries_ShouldNotEvictSeriesReownedBeforeGracePeriodButNotReconciled
+// covers a race seen in production during a rapid partition-count change: a
+// series is queued as non-owned, ownership flips back before the owned-series
+// service's own ticker gets a chance to reconcile pendingNonOwnedRefs, and a
+// fresh sample lands for the series in the meantime. Before the fix,
+// compactBlocksDueToNonOwnedSeries trusted the stale queue entry and evicted
+// the series -- fresh sample included -- once the grace period elapsed, with
+// no re-check of current ownership. This test proves the fix's pre-eviction
+// re-check catches that: the series survives in the head.
+func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldNotEvictSeriesReownedBeforeGracePeriodButNotReconciled(t *testing.T) {
+	var (
+		ctx         = context.Background()
+		ctxWithUser = user.InjectOrgID(ctx, userID)
+	)
+
+	cfg := defaultIngesterTestConfig(t)
+	cfg.BlocksStorageConfig.TSDB.HeadCompactionInterval = time.Hour
+	cfg.UpdateIngesterOwnedSeries = true
+	cfg.EarlyCompactionNonOwnedSeriesEnabled = true
+	// A long min grace period that no real-time elapse can cross during the test; max grace
+	// period is disabled so only the min-grace path is exercised, exactly as in
+	// TestIngester_compactBlocksDueToNonOwnedSeries_ShouldRespectGracePeriod above.
+	cfg.EarlyCompactionNonOwnedSeriesMinGracePeriod = time.Hour
+	cfg.EarlyCompactionNonOwnedSeriesMaxGracePeriod = 0
+
+	limits := defaultLimitsTestConfig()
+	limits.EarlyHeadCompactionOwnedSeriesThreshold = 1
+
+	ingesters := setupTestIngesterRing(t, []string{"zone-a", "zone-b", "zone-c"}, 1, cfg, limits)
+	ingester := ingesters[0]
+
+	sampleTime, err := time.Parse(time.RFC3339, "2026-05-05T00:00:00Z")
+	require.NoError(t, err)
+	t1 := sampleTime.UnixMilli()
+	t2 := t1 + 1
+
+	ownedLabels, reshardedLabels, minHash := pickOwnedAndNonOwnedSeries(t, userID)
+
+	for _, lbls := range []labels.Labels{ownedLabels, reshardedLabels} {
+		require.NoError(t, pushSeriesToIngester(ctxWithUser, t, ingester, []util_test.Series{{
+			Labels:  lbls,
+			Samples: []util_test.Sample{{TS: t1, Val: 1.0}},
+		}}))
+		require.NoError(t, pushSeriesToIngester(ctxWithUser, t, ingester, []util_test.Series{{
+			Labels:  lbls,
+			Samples: []util_test.Sample{{TS: t2, Val: 2.0}},
+		}}))
+	}
+
+	db := ingester.getTSDB(userID)
+	require.NotNil(t, db)
+	require.Equal(t, uint64(2), db.Head().NumSeries())
+
+	// A partition-count change puts reshardedLabels outside the owned range, so
+	// recomputeOwnedSeries queues it as pending non-owned.
+	db.ownedTokenRanges = ring.TokenRanges{0, minHash}
+	require.True(t, db.recomputeOwnedSeries(0, "ring changed", log.NewNopLogger()))
+	require.Equal(t, 1, db.ownedSeriesState().ownedSeriesCount, "exactly one series should be owned right after the resize")
+
+	db.pendingNonOwnedRefsMtx.Lock()
+	require.Len(t, db.pendingNonOwnedRefs, 1, "the resharded series should be queued as pending non-owned")
+	db.pendingNonOwnedRefsMtx.Unlock()
+
+	// Ownership flips back, but we deliberately skip recomputeOwnedSeries here --
+	// simulating the reconciling tick not having run yet.
+	db.ownedTokenRanges = ring.TokenRanges{0, math.MaxUint32}
+
+	// A fresh write lands for reshardedLabels, as it legitimately would now that
+	// it's owned again.
+	t3 := t2 + 1
+	require.NoError(t, pushSeriesToIngester(ctxWithUser, t, ingester, []util_test.Series{{
+		Labels:  reshardedLabels,
+		Samples: []util_test.Sample{{TS: t3, Val: 3.0}},
+	}}))
+	require.Equal(t, uint64(2), db.Head().NumSeries(), "the fresh write should still be visible in the head before eviction runs")
+
+	// Backdate the pending entry so its grace period has elapsed.
+	backdated := time.Now().Add(-2 * time.Hour)
+	db.pendingNonOwnedRefsMtx.Lock()
+	for r := range db.pendingNonOwnedRefs {
+		db.pendingNonOwnedRefs[r] = backdated
+	}
+	db.pendingNonOwnedRefsMtx.Unlock()
+
+	userBlocksDir := filepath.Join(ingester.cfg.BlocksStorageConfig.TSDB.Dir, userID)
+
+	// Run eviction. The fix's pre-check reconciles reshardedLabels out of the
+	// queue first, so it isn't evicted -- both series, including the fresh
+	// sample, survive.
+	ingester.compactBlocksDueToNonOwnedSeries(ctx, 0)
+
+	require.Empty(t, listBlocksInDir(t, userBlocksDir), "no block should be produced: the presently-owned series must not be evicted")
+	require.Equal(t, uint64(2), db.Head().NumSeries(),
+		"the presently-owned, just-written series must survive in the head")
+
+	db.pendingNonOwnedRefsMtx.Lock()
+	require.Empty(t, db.pendingNonOwnedRefs, "the re-owned series must be reconciled out of pendingNonOwnedRefs")
+	db.pendingNonOwnedRefsMtx.Unlock()
+}
+
 // TestIngester_compactBlocksDueToNonOwnedSeries_ShouldEvictAgedRefsDespiteFresherOnes verifies
 // that pending non-owned refs are evicted based on their individual grace periods.
 //

From caec8593c2b74951f963e2e1f30566f1485307be Mon Sep 17 00:00:00 2001
From: Yuri Nikolic <durica.nikolic@grafana.com>
Date: Mon, 7 Sep 2026 23:47:24 +0200
Subject: [PATCH 2/4] Ingester: re-validate ownership before evicting non-owned
 series

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
---
 CHANGELOG.md | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 91cd91f0694..bf85aac595d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -48,6 +48,7 @@
 * [BUGFIX] Mimir: Exit with status 0, and stop logging `module failed` at error level, when a `SIGTERM` or `SIGINT` arrives before all modules finished starting. Cancelling the start context leaves those modules in a failed state, which was reported as `failed services` and exited 1, making an ordinary rolling restart or node drain of a slow-starting component indistinguishable from a crash. #16524
 * [BUGFIX] Build: Use `#!/usr/bin/env bash`/`#!/usr/bin/env sh` instead of hardcoded interpreter paths in development and CI scripts, fixing failures on systems where those interpreters aren't at that exact path, such as NixOS. #16425
 * [BUGFIX] Query-scheduler: Fix a data race that could crash the query-scheduler when gRPC client cluster validation is enabled. The scheduler builds gRPC dial options per request from concurrent querier loops, and the shared client configuration wrote the cluster validation interceptor back onto itself, so those requests raced on the same field. #16531
+* [BUGFIX] Ingester: Fix a race in early compaction of non-owned series (`-ingester.early-compaction-non-owned-series-enabled`) where a series that became owned again after being queued for eviction, but before the owned-series service reconciled the queue, could still be evicted once its grace period elapsed — silently dropping any samples written to it after ownership was restored. `compactBlocksDueToNonOwnedSeries` now re-validates ownership immediately before evicting queued series. #16534
 
 ### Mixin
 
@@ -251,9 +252,6 @@
 * [ENHANCEMENT] Make range vector splitting configurable per query path. #15706
 * [ENHANCEMENT] Add `newMimirtoolBlocksJob` and subcommand-specific helpers to run `mimirtool blocks` as Kubernetes Jobs. #15757
 * [BUGFIX] Continuous-test: Include `._config.commonConfig` in arguments passed to continuous-test. #15988
-* [BUGFIX] Add missing `-querier.mimir-query-engine.range-vector-splitting.memcached.addresses` to `multi_zone_config_validation_excluded_args`. #16237
-* [BUGFIX] Ingester: Fix a race in early compaction of non-owned series (`-ingester.early-compaction-non-owned-series-enabled`) where a series that became owned again after being queued for eviction, but before the owned-series service reconciled the queue, could still be evicted once its grace period elapsed — silently dropping any samples written to it after ownership was restored. `compactBlocksDueToNonOwnedSeries` now re-validates ownership immediately before evicting queued series. #XXXXX
-
 
 ### Documentation
 

From 7bc07db9c157bfe8f03f018e474070a73bae5386 Mon Sep 17 00:00:00 2001
From: Yuri Nikolic <durica.nikolic@grafana.com>
Date: Tue, 8 Sep 2026 11:45:55 +0200
Subject: [PATCH 3/4] Fixing Cursor findings

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
---
 pkg/ingester/ingester_compaction.go           |  33 +++---
 .../ingester_early_compaction_test.go         | 112 +++++++++++++++---
 pkg/ingester/user_tsdb.go                     |  42 ++++++-
 pkg/ingester/user_tsdb_test.go                |  72 +++++++++++
 4 files changed, 219 insertions(+), 40 deletions(-)

diff --git a/pkg/ingester/ingester_compaction.go b/pkg/ingester/ingester_compaction.go
index 6cd62211bd4..b9126e40435 100644
--- a/pkg/ingester/ingester_compaction.go
+++ b/pkg/ingester/ingester_compaction.go
@@ -630,21 +630,24 @@ func (i *Ingester) compactBlocksDueToNonOwnedSeries(ctx context.Context, jitter
 			continue
 		}
 
-		// Ownership can flip back to this ingester after a series was queued as
-		// non-owned, but before the owned-series service's own ticker notices and
-		// reconciles pendingNonOwnedRefs — this loop runs on a separate schedule
-		// with no guarantee it runs after that reconciliation. Re-running
-		// computeOwnedSeries() here catches that: series that are owned again get
-		// removed from pendingNonOwnedRefs before takePendingNonOwnedRefs can evict
-		// them below. We reuse the ranges already cached on this userTSDB rather
-		// than asking the ring for fresh ones — that's the owned-series service's
-		// job — so this is a no-op when ownership hasn't actually changed.
-		db.pendingNonOwnedRefsMtx.Lock()
-		hasPendingNonOwnedRefs := len(db.pendingNonOwnedRefs) > 0
-		db.pendingNonOwnedRefsMtx.Unlock()
-
-		if hasPendingNonOwnedRefs {
-			db.recomputeOwnedSeries(db.ownedSeriesState().shardSize, "early compaction pre-eviction re-check", i.logger)
+		// A series queued as non-owned can become owned again before the owned-series
+		// service's own ticker reconciles pendingNonOwnedRefs, since that ticker runs
+		// on its own schedule. Re-checking against the cached ownedTokenRanges won't
+		// catch this: the cache is only ever updated by updateTenant, which reconciles
+		// pendingNonOwnedRefs in that same call, so re-scanning a stale cache just
+		// repeats the same stale answer. Calling updateTenant here instead re-fetches
+		// current ranges from the ring, so a series owned again is removed from
+		// pendingNonOwnedRefs before takePendingNonOwnedRefs can evict it below.
+		// updateTenant short-circuits when ranges haven't changed, so this is cheap
+		// unless there's an actual discrepancy to reconcile.
+		if i.ownedSeriesService != nil {
+			db.pendingNonOwnedRefsMtx.Lock()
+			hasPendingNonOwnedRefs := len(db.pendingNonOwnedRefs) > 0
+			db.pendingNonOwnedRefsMtx.Unlock()
+
+			if hasPendingNonOwnedRefs {
+				i.ownedSeriesService.updateTenant(userID, db, true)
+			}
 		}
 
 		now := time.Now()
diff --git a/pkg/ingester/ingester_early_compaction_test.go b/pkg/ingester/ingester_early_compaction_test.go
index 6e99ae1a419..281ee4d8b17 100644
--- a/pkg/ingester/ingester_early_compaction_test.go
+++ b/pkg/ingester/ingester_early_compaction_test.go
@@ -16,6 +16,7 @@ import (
 	"github.com/go-kit/log"
 	"github.com/grafana/dskit/kv/consul"
 	"github.com/grafana/dskit/ring"
+	"github.com/grafana/dskit/services"
 	"github.com/grafana/dskit/user"
 	"github.com/oklog/ulid/v2"
 	"github.com/prometheus/common/model"
@@ -1052,6 +1053,11 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldFlushDataToBlock(t *tes
 	ingesters := setupTestIngesterRing(t, []string{"zone-a", "zone-b", "zone-c"}, 1, cfg, limits)
 	ingester := ingesters[0]
 
+	// Install a fake ring strategy agreeing with the ranges below, so compactBlocksDueToNonOwnedSeries's
+	// ring re-check is a no-op instead of the real (single-ingester, full-ownership) ring overriding it.
+	ownedLabels, nonOwnedLabels, minHash := pickOwnedAndNonOwnedSeries(t, userID)
+	installFakeOwnedSeriesRingStrategy(t, ingester, ring.TokenRanges{0, minHash}, 0)
+
 	// Push two samples at different timestamps to ensure the head's MinTime is less
 	// than MaxTime. Use two series so one remains owned (satisfying the per-tenant
 	// gate) while the other is marked non-owned and evicted.
@@ -1060,7 +1066,6 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldFlushDataToBlock(t *tes
 	t1 := sampleTime.UnixMilli()
 	t2 := t1 + 1
 
-	ownedLabels, nonOwnedLabels, minHash := pickOwnedAndNonOwnedSeries(t, userID)
 	nonOwnedName := nonOwnedLabels.Get(model.MetricNameLabel)
 	nonOwnedMetricModel := model.Metric{model.MetricNameLabel: model.LabelValue(nonOwnedName)}
 
@@ -1167,6 +1172,8 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldFlushOnlyNonOwnedSeries
 	ownedName := ownedLabels.Get(model.MetricNameLabel)
 	nonOwnedName := nonOwnedLabels.Get(model.MetricNameLabel)
 
+	installFakeOwnedSeriesRingStrategy(t, ingester, ring.TokenRanges{0, minHash}, 0)
+
 	for _, lbls := range []labels.Labels{ownedLabels, nonOwnedLabels} {
 		require.NoError(t, pushSeriesToIngester(ctxWithUser, t, ingester, []util_test.Series{{
 			Labels:  lbls,
@@ -1275,6 +1282,8 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldHandleOOOSamples(t *tes
 	ownedName := ownedLabels.Get(model.MetricNameLabel)
 	nonOwnedName := nonOwnedLabels.Get(model.MetricNameLabel)
 
+	installFakeOwnedSeriesRingStrategy(t, ingester, ring.TokenRanges{0, minHash}, 0)
+
 	// For each series, push two in-order samples at t1 and t2, then an
 	// out-of-order sample at tOOO so the series enters the OOO ingestion path and
 	// acquires in-memory OOO state (s.ooo becomes non-nil).
@@ -1413,6 +1422,8 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldRespectGracePeriod(t *t
 	// other is non-owned and exercises the grace-period gating.
 	ownedLabels, nonOwnedLabels, minHash := pickOwnedAndNonOwnedSeries(t, userID)
 
+	installFakeOwnedSeriesRingStrategy(t, ingester, ring.TokenRanges{0, minHash}, 0)
+
 	for _, lbls := range []labels.Labels{ownedLabels, nonOwnedLabels} {
 		require.NoError(t, pushSeriesToIngester(ctxWithUser, t, ingester, []util_test.Series{{
 			Labels:  lbls,
@@ -1519,6 +1530,7 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldHandleScaleUp(t *testin
 		require.Equal(t, uint64(numSeries), db.Head().NumSeries(), "all series should be in the head")
 
 		db.ownedTokenRanges = ring.TokenRanges{0, math.MaxUint32 / uint32(ingestersPerZone)}
+		installFakeOwnedSeriesRingStrategy(t, ingester, db.ownedTokenRanges, 0)
 		require.True(t, db.recomputeOwnedSeries(0, "test", log.NewNopLogger()), "recomputeOwnedSeries should succeed")
 		ownedAfterRecompute := db.ownedSeriesState().ownedSeriesCount
 		require.Less(t, ownedAfterRecompute, numSeries, "some series should be non-owned")
@@ -1655,6 +1667,7 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_StaleRefsAfterPriorEviction(t
 
 	// Configure ownership so one series is non-owned and gets queued in pendingNonOwnedRefs.
 	db.ownedTokenRanges = ring.TokenRanges{0, minHash}
+	installFakeOwnedSeriesRingStrategy(t, ingester, ring.TokenRanges{0, minHash}, 0)
 	require.True(t, db.recomputeOwnedSeries(0, "test", log.NewNopLogger()))
 	require.Equal(t, 1, db.ownedSeriesState().ownedSeriesCount)
 
@@ -1698,14 +1711,16 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_StaleRefsAfterPriorEviction(t
 }
 
 // TestIngester_compactBlocksDueToNonOwnedSeries_ShouldNotEvictSeriesReownedBeforeGracePeriodButNotReconciled
-// covers a race seen in production during a rapid partition-count change: a
-// series is queued as non-owned, ownership flips back before the owned-series
-// service's own ticker gets a chance to reconcile pendingNonOwnedRefs, and a
-// fresh sample lands for the series in the meantime. Before the fix,
-// compactBlocksDueToNonOwnedSeries trusted the stale queue entry and evicted
-// the series -- fresh sample included -- once the grace period elapsed, with
-// no re-check of current ownership. This test proves the fix's pre-eviction
-// re-check catches that: the series survives in the head.
+// covers a rapid partition-count change: a series is queued as non-owned, ownership flips back
+// before the owned-series service's ticker reconciles pendingNonOwnedRefs, and a fresh sample
+// lands for it in the meantime. Before the fix, compactBlocksDueToNonOwnedSeries trusted the
+// stale queue entry and evicted the series -- fresh sample included -- once the grace period
+// elapsed.
+//
+// The ring change goes through a fakeOwnedSeriesRingStrategy rather than a direct write to
+// db.ownedTokenRanges: updateTenant only reconciles pendingNonOwnedRefs when the ranges it
+// fetches differ from the cache, so the test has to go through that path (ring says X, then
+// ring says Y) for the fix's re-check to have anything real to catch.
 func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldNotEvictSeriesReownedBeforeGracePeriodButNotReconciled(t *testing.T) {
 	var (
 		ctx         = context.Background()
@@ -1750,19 +1765,22 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldNotEvictSeriesReownedBe
 	require.NotNil(t, db)
 	require.Equal(t, uint64(2), db.Head().NumSeries())
 
-	// A partition-count change puts reshardedLabels outside the owned range, so
-	// recomputeOwnedSeries queues it as pending non-owned.
-	db.ownedTokenRanges = ring.TokenRanges{0, minHash}
-	require.True(t, db.recomputeOwnedSeries(0, "ring changed", log.NewNopLogger()))
+	// A partition-count change puts reshardedLabels outside the owned range. Install a fake
+	// ring strategy reporting that, then run updateTenant -- exactly the owned-series service's
+	// own tick -- so it fetches those ranges, updates the cache, and queues reshardedLabels as
+	// pending non-owned.
+	strategy := installFakeOwnedSeriesRingStrategy(t, ingester, ring.TokenRanges{0, minHash}, 0)
+	require.True(t, ingester.ownedSeriesService.updateTenant(userID, db, true))
 	require.Equal(t, 1, db.ownedSeriesState().ownedSeriesCount, "exactly one series should be owned right after the resize")
 
 	db.pendingNonOwnedRefsMtx.Lock()
 	require.Len(t, db.pendingNonOwnedRefs, 1, "the resharded series should be queued as pending non-owned")
 	db.pendingNonOwnedRefsMtx.Unlock()
 
-	// Ownership flips back, but we deliberately skip recomputeOwnedSeries here --
-	// simulating the reconciling tick not having run yet.
-	db.ownedTokenRanges = ring.TokenRanges{0, math.MaxUint32}
+	// The ring genuinely changes back, so reshardedLabels is owned again. Update only the fake
+	// strategy -- not the cache, and deliberately without calling updateTenant again -- to
+	// simulate the reconciling tick not having run yet even though the ring has already moved on.
+	strategy.setRanges(ring.TokenRanges{0, math.MaxUint32})
 
 	// A fresh write lands for reshardedLabels, as it legitimately would now that
 	// it's owned again.
@@ -1783,9 +1801,9 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldNotEvictSeriesReownedBe
 
 	userBlocksDir := filepath.Join(ingester.cfg.BlocksStorageConfig.TSDB.Dir, userID)
 
-	// Run eviction. The fix's pre-check reconciles reshardedLabels out of the
-	// queue first, so it isn't evicted -- both series, including the fresh
-	// sample, survive.
+	// Run eviction. The fix's pre-check calls updateTenant, which now sees the fake strategy's
+	// ranges no longer match what's cached, reconciles reshardedLabels out of the queue, and
+	// both series -- including the fresh sample -- survive.
 	ingester.compactBlocksDueToNonOwnedSeries(ctx, 0)
 
 	require.Empty(t, listBlocksInDir(t, userBlocksDir), "no block should be produced: the presently-owned series must not be evicted")
@@ -1867,6 +1885,7 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldEvictAgedRefsDespiteFre
 	// Empty owned-token-ranges marks every head series as non-owned. recomputeOwnedSeries then
 	// stamps each ref's pending timestamp with time.Now().
 	db.ownedTokenRanges = ring.TokenRanges{}
+	installFakeOwnedSeriesRingStrategy(t, ingester, ring.TokenRanges{}, 0)
 	require.True(t, db.recomputeOwnedSeries(0, "test", log.NewNopLogger()), "recomputeOwnedSeries should succeed")
 	require.Equal(t, 0, db.ownedSeriesState().ownedSeriesCount, "no series should be owned")
 
@@ -1936,6 +1955,61 @@ func pickOwnedAndNonOwnedSeries(t *testing.T, userID string) (ownedLabels, nonOw
 	return labelsB, labelsA, hashB
 }
 
+// fakeOwnedSeriesRingStrategy is a directly-controllable ownedSeriesRingStrategy, letting tests
+// dictate what "the ring currently says" instead of needing a real multi-ingester ring. Call
+// setRanges mid-test to simulate a ring change.
+type fakeOwnedSeriesRingStrategy struct {
+	mu     sync.Mutex
+	ranges ring.TokenRanges
+	shard  int
+}
+
+func (f *fakeOwnedSeriesRingStrategy) checkRingForChanges() (bool, error) { return true, nil }
+
+func (f *fakeOwnedSeriesRingStrategy) shardSizeForUser(_ string) int {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return f.shard
+}
+
+func (f *fakeOwnedSeriesRingStrategy) tokenRangesForUser(_ string, _ int) (ring.TokenRanges, error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return f.ranges, nil
+}
+
+func (f *fakeOwnedSeriesRingStrategy) ownerKeyAndValue() (string, string) {
+	return "fake_ring_strategy", "test"
+}
+
+func (f *fakeOwnedSeriesRingStrategy) setRanges(ranges ring.TokenRanges) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	f.ranges = ranges
+}
+
+// installFakeOwnedSeriesRingStrategy stops the ingester's real owned-series service, which would
+// otherwise keep ticking in the background and race whatever the test sets up, and replaces it
+// with one backed by a fakeOwnedSeriesRingStrategy reporting initialRanges. The replacement is
+// never started: tests call recomputeOwnedSeries/updateTenant on it directly.
+func installFakeOwnedSeriesRingStrategy(t *testing.T, ingester *Ingester, initialRanges ring.TokenRanges, shardSize int) *fakeOwnedSeriesRingStrategy {
+	t.Helper()
+
+	require.NoError(t, services.StopAndAwaitTerminated(context.Background(), ingester.ownedSeriesService))
+
+	strategy := &fakeOwnedSeriesRingStrategy{ranges: initialRanges, shard: shardSize}
+	ingester.ownedSeriesService = newOwnedSeriesService(
+		time.Hour, // Tests drive recompute directly; this service is never started.
+		strategy,
+		log.NewNopLogger(),
+		nil,
+		ingester.limiter.maxSeriesPerUser,
+		ingester.getTSDBUsers,
+		ingester.getTSDB,
+	)
+	return strategy
+}
+
 func setupTestIngesterRing(t *testing.T, zones []string, ingestersPerZone int, cfg Config, limitsCfg validation.Limits) []*Ingester {
 	// Create a shared consul KV store so all ingesters join the same ring.
 	consulClient, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger(), nil)
diff --git a/pkg/ingester/user_tsdb.go b/pkg/ingester/user_tsdb.go
index 8ffc16d961a..10b7eb7c646 100644
--- a/pkg/ingester/user_tsdb.go
+++ b/pkg/ingester/user_tsdb.go
@@ -144,8 +144,16 @@ type userTSDB struct {
 	ownedStateMtx sync.Mutex
 	ownedState    ownedSeriesState
 
-	// Only accessed by ownedSeries service, no need to synchronization.
-	ownedTokenRanges ring.TokenRanges
+	// recomputeOwnedSeriesMtx serializes recompute calls (recomputeOwnedSeries /
+	// recomputeOwnedSeriesWithComputeFn) for this tenant. Both the owned-series service and the
+	// compaction loop's pre-eviction ownership re-check can call these concurrently; without this
+	// lock, a slower call could overwrite a faster one's more up-to-date ownedState.
+	recomputeOwnedSeriesMtx sync.Mutex
+
+	// ownedTokenRangesMtx guards ownedTokenRanges for the same reason: updateTokenRanges (write)
+	// and computeOwnedSeries (read) can now both be called from either of those two goroutines.
+	ownedTokenRangesMtx sync.Mutex
+	ownedTokenRanges    ring.TokenRanges
 
 	// offsetCatalogue tracks Kafka offset watermarks for compacted blocks.
 	// Only set when ingest storage is enabled.
@@ -614,7 +622,8 @@ func (u *userTSDB) triggerRecomputeOwnedSeries(reason string) {
 // This method returns false, if recomputation of owned series failed multiple times due to too
 // many new series being added during the computation. If no such problem happened, this method returns true.
 //
-// This method and updateTokenRanges should be only called from the same goroutine. (ownedSeries service)
+// Safe to call from any goroutine: concurrent calls for the same tenant are serialized by
+// recomputeOwnedSeriesMtx.
 func (u *userTSDB) recomputeOwnedSeries(shardSize int, reason string, logger log.Logger) (success bool) {
 	success, _ = u.recomputeOwnedSeriesWithComputeFn(shardSize, reason, logger, u.computeOwnedSeries)
 	return success
@@ -626,6 +635,12 @@ const (
 )
 
 func (u *userTSDB) recomputeOwnedSeriesWithComputeFn(shardSize int, reason string, logger log.Logger, compute func() int) (success bool, _ int) {
+	// Only one recompute runs at a time for this tenant. Without this, a concurrent call
+	// (from the compaction loop's pre-eviction re-check, racing the owned-series service's
+	// own tick) could interleave with this one and overwrite ownedState with a stale result.
+	u.recomputeOwnedSeriesMtx.Lock()
+	defer u.recomputeOwnedSeriesMtx.Unlock()
+
 	start := time.Now()
 
 	var ownedSeriesNew, ownedSeriesBefore, shardSizeBefore, localLimitBefore, localLimitNew int
@@ -685,14 +700,24 @@ func (u *userTSDB) recomputeOwnedSeriesWithComputeFn(shardSize int, reason strin
 
 // updateTokenRanges sets owned token ranges to supplied value, and returns true, if token ranges have changed.
 //
-// This method and recomputeOwnedSeries should be only called from the same goroutine. (ownedSeries service)
+// Safe to call from any goroutine.
 func (u *userTSDB) updateTokenRanges(newTokenRanges []uint32) bool {
+	u.ownedTokenRangesMtx.Lock()
 	prev := u.ownedTokenRanges
 	u.ownedTokenRanges = newTokenRanges
+	u.ownedTokenRangesMtx.Unlock()
 
 	return !prev.Equal(newTokenRanges)
 }
 
+// getOwnedTokenRanges returns the tenant's current owned token ranges. Safe to call from any
+// goroutine.
+func (u *userTSDB) getOwnedTokenRanges() ring.TokenRanges {
+	u.ownedTokenRangesMtx.Lock()
+	defer u.ownedTokenRangesMtx.Unlock()
+	return u.ownedTokenRanges
+}
+
 // addPendingNonOwnedRefs reconciles the per-tenant pending-eviction set with the
 // caller's authoritative snapshot of currently non-owned refs.
 //
@@ -756,10 +781,15 @@ func (u *userTSDB) takePendingNonOwnedRefs(notAfter time.Time) []storage.SeriesR
 }
 
 func (u *userTSDB) computeOwnedSeries() int {
+	// Snapshot once and reuse for the whole scan below: updateTokenRanges only ever replaces the
+	// ranges wholesale, never mutates them in place, so one snapshot is safe to reuse and gives
+	// every series in this pass a consistent view.
+	ownedTokenRanges := u.getOwnedTokenRanges()
+
 	// If no token ranges are assigned, all head series are non-owned.
 	// activeSeries.Clear handles the active-series state; the loop below collects
 	// refs for targeted eviction.
-	allNonOwned := len(u.ownedTokenRanges) == 0
+	allNonOwned := len(ownedTokenRanges) == 0
 	if allNonOwned {
 		u.activeSeries.Clear()
 	}
@@ -788,7 +818,7 @@ func (u *userTSDB) computeOwnedSeries() int {
 			return
 		}
 		for i, sh := range secondaryHashes {
-			if u.ownedTokenRanges.IncludesKey(sh) {
+			if ownedTokenRanges.IncludesKey(sh) {
 				count++
 				continue
 			}
diff --git a/pkg/ingester/user_tsdb_test.go b/pkg/ingester/user_tsdb_test.go
index cabbe430c64..3d3f53b4b5b 100644
--- a/pkg/ingester/user_tsdb_test.go
+++ b/pkg/ingester/user_tsdb_test.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"math"
 	"slices"
+	"sync"
 	"testing"
 	"time"
 
@@ -669,6 +670,77 @@ func TestRecomputeOwnedSeries(t *testing.T) {
 	})
 }
 
+// TestRecomputeOwnedSeries_ConcurrentWithUpdateTokenRanges guards against a race in the
+// compaction loop's pre-eviction ownership re-check: recomputeOwnedSeries and updateTokenRanges
+// are now callable from two goroutines (owned-series service and compaction loop), so this runs
+// both concurrently to prove ownedTokenRanges and ownedState stay properly synchronized.
+//
+// No assertions by design -- verification is the race detector, not a value check, so this only
+// means something under `go test -race` (Mimir's CI default for this package). Removing either
+// of userTSDB's two locks (ownedTokenRangesMtx, recomputeOwnedSeriesMtx) reproduces the race
+// under -race.
+func TestRecomputeOwnedSeries_ConcurrentWithUpdateTokenRanges(t *testing.T) {
+	const userID = "test-user"
+	limits := validation.Limits{MaxGlobalSeriesPerUser: 0}
+	overrides := validation.NewOverrides(limits, nil)
+	limiter := NewLimiter(overrides, newIngesterRingLimiterStrategy(nil, 3, true, "zone", overrides.IngestionTenantShardSize))
+
+	opts := tsdb.DefaultOptions()
+	opts.SecondaryHashFunction = secondaryTSDBHashFunctionForUser(userID)
+	tsdbDB, err := tsdb.Open(t.TempDir(), promslog.NewNopLogger(), nil, opts, nil)
+	require.NoError(t, err)
+	t.Cleanup(func() { require.NoError(t, tsdbDB.Close()) })
+
+	app := tsdbDB.Appender(context.Background())
+	for i := 0; i < 100; i++ {
+		_, err := app.Append(0, labels.FromStrings("__name__", fmt.Sprintf("metric_%d", i)), 100, 1.0)
+		require.NoError(t, err)
+	}
+	require.NoError(t, app.Commit())
+
+	db := &userTSDB{
+		userID:       userID,
+		cfg:          &Config{EarlyCompactionNonOwnedSeriesEnabled: true},
+		db:           tsdbDB,
+		limiter:      limiter,
+		activeSeries: activeseries.NewActiveSeries(asmodel.NewMatchers(asmodel.CustomTrackersConfig{}), time.Minute, nil),
+	}
+
+	const iterations = 200
+	var wg sync.WaitGroup
+	wg.Add(3)
+
+	// Simulates the owned-series service's own goroutine repeatedly updating ranges as the
+	// ring changes.
+	go func() {
+		defer wg.Done()
+		for i := 0; i < iterations; i++ {
+			if i%2 == 0 {
+				db.updateTokenRanges(ring.TokenRanges{0, math.MaxUint32})
+			} else {
+				db.updateTokenRanges(ring.TokenRanges{0, math.MaxUint32 / 2})
+			}
+		}
+	}()
+
+	// Two goroutines both call recomputeOwnedSeries concurrently: one simulates the
+	// owned-series service's own tick, the other simulates the compaction loop's
+	// pre-eviction re-check landing at the same time. This is what exercises the
+	// ownedState lost-update race specifically (two concurrent recomputes racing each
+	// other), separately from the ownedTokenRanges race exercised by the goroutine above.
+	for shardSize := 3; shardSize <= 4; shardSize++ {
+		shardSize := shardSize
+		go func() {
+			defer wg.Done()
+			for i := 0; i < iterations; i++ {
+				db.recomputeOwnedSeries(shardSize, "test", log.NewNopLogger())
+			}
+		}()
+	}
+
+	wg.Wait()
+}
+
 // BenchmarkUserTSDB_addPendingNonOwnedRefs measures the per-call cost of the
 // reconciliation logic across the four regimes the production path exercises:
 //   - "fresh": empty set, snapshot of N refs (first detection after a ring change).

From c05142efa84bf5ec4c874f9f9947181fe02c09e8 Mon Sep 17 00:00:00 2001
From: Yuri Nikolic <durica.nikolic@grafana.com>
Date: Tue, 8 Sep 2026 12:03:42 +0200
Subject: [PATCH 4/4] Fixing Cursor findings

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
---
 pkg/ingester/ingester_compaction.go           |  10 +-
 .../ingester_early_compaction_test.go         | 100 +++++++++++++++++-
 pkg/ingester/owned_series.go                  |  24 +++--
 3 files changed, 123 insertions(+), 11 deletions(-)

diff --git a/pkg/ingester/ingester_compaction.go b/pkg/ingester/ingester_compaction.go
index b9126e40435..56086098b6a 100644
--- a/pkg/ingester/ingester_compaction.go
+++ b/pkg/ingester/ingester_compaction.go
@@ -646,7 +646,15 @@ func (i *Ingester) compactBlocksDueToNonOwnedSeries(ctx context.Context, jitter
 			db.pendingNonOwnedRefsMtx.Unlock()
 
 			if hasPendingNonOwnedRefs {
-				i.ownedSeriesService.updateTenant(userID, db, true)
+				if _, err := i.ownedSeriesService.updateTenant(userID, db, true); err != nil {
+					// The ring lookup failed, so pendingNonOwnedRefs was NOT reconciled this
+					// round: it may still contain refs that are owned again. Ring lookups are
+					// especially likely to fail during the same ring instability that causes
+					// ownership to flip in the first place, so skip eviction for this tenant
+					// rather than risk evicting a currently-owned series. updateTenant already
+					// scheduled a retry for the next tick.
+					continue
+				}
 			}
 		}
 
diff --git a/pkg/ingester/ingester_early_compaction_test.go b/pkg/ingester/ingester_early_compaction_test.go
index 281ee4d8b17..3756c17890a 100644
--- a/pkg/ingester/ingester_early_compaction_test.go
+++ b/pkg/ingester/ingester_early_compaction_test.go
@@ -1770,7 +1770,9 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldNotEvictSeriesReownedBe
 	// own tick -- so it fetches those ranges, updates the cache, and queues reshardedLabels as
 	// pending non-owned.
 	strategy := installFakeOwnedSeriesRingStrategy(t, ingester, ring.TokenRanges{0, minHash}, 0)
-	require.True(t, ingester.ownedSeriesService.updateTenant(userID, db, true))
+	updated, err := ingester.ownedSeriesService.updateTenant(userID, db, true)
+	require.NoError(t, err)
+	require.True(t, updated)
 	require.Equal(t, 1, db.ownedSeriesState().ownedSeriesCount, "exactly one series should be owned right after the resize")
 
 	db.pendingNonOwnedRefsMtx.Lock()
@@ -1815,6 +1817,90 @@ func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldNotEvictSeriesReownedBe
 	db.pendingNonOwnedRefsMtx.Unlock()
 }
 
+// TestIngester_compactBlocksDueToNonOwnedSeries_ShouldSkipEvictionWhenRingLookupFails covers a
+// ring lookup failing during the pre-eviction re-check: updateTenant returns without
+// reconciling pendingNonOwnedRefs, so compactBlocksDueToNonOwnedSeries cannot trust the queue is
+// up to date and must not evict from it, since a series that's owned again could be sitting in
+// there unreconciled.
+func TestIngester_compactBlocksDueToNonOwnedSeries_ShouldSkipEvictionWhenRingLookupFails(t *testing.T) {
+	var (
+		ctx         = context.Background()
+		ctxWithUser = user.InjectOrgID(ctx, userID)
+	)
+
+	cfg := defaultIngesterTestConfig(t)
+	cfg.BlocksStorageConfig.TSDB.HeadCompactionInterval = time.Hour
+	cfg.UpdateIngesterOwnedSeries = true
+	cfg.EarlyCompactionNonOwnedSeriesEnabled = true
+	cfg.EarlyCompactionNonOwnedSeriesMinGracePeriod = time.Hour
+	cfg.EarlyCompactionNonOwnedSeriesMaxGracePeriod = 0
+
+	limits := defaultLimitsTestConfig()
+	limits.EarlyHeadCompactionOwnedSeriesThreshold = 1
+
+	ingesters := setupTestIngesterRing(t, []string{"zone-a", "zone-b", "zone-c"}, 1, cfg, limits)
+	ingester := ingesters[0]
+
+	sampleTime, err := time.Parse(time.RFC3339, "2026-05-05T00:00:00Z")
+	require.NoError(t, err)
+	t1 := sampleTime.UnixMilli()
+	t2 := t1 + 1
+
+	ownedLabels, reshardedLabels, minHash := pickOwnedAndNonOwnedSeries(t, userID)
+
+	for _, lbls := range []labels.Labels{ownedLabels, reshardedLabels} {
+		require.NoError(t, pushSeriesToIngester(ctxWithUser, t, ingester, []util_test.Series{{
+			Labels:  lbls,
+			Samples: []util_test.Sample{{TS: t1, Val: 1.0}},
+		}}))
+		require.NoError(t, pushSeriesToIngester(ctxWithUser, t, ingester, []util_test.Series{{
+			Labels:  lbls,
+			Samples: []util_test.Sample{{TS: t2, Val: 2.0}},
+		}}))
+	}
+
+	db := ingester.getTSDB(userID)
+	require.NotNil(t, db)
+	require.Equal(t, uint64(2), db.Head().NumSeries())
+
+	// A partition-count change puts reshardedLabels outside the owned range; queue it as
+	// pending non-owned via a successful updateTenant call, exactly as the owned-series
+	// service's own tick would.
+	strategy := installFakeOwnedSeriesRingStrategy(t, ingester, ring.TokenRanges{0, minHash}, 0)
+	updated, err := ingester.ownedSeriesService.updateTenant(userID, db, true)
+	require.NoError(t, err)
+	require.True(t, updated)
+
+	db.pendingNonOwnedRefsMtx.Lock()
+	require.Len(t, db.pendingNonOwnedRefs, 1, "the resharded series should be queued as pending non-owned")
+	db.pendingNonOwnedRefsMtx.Unlock()
+
+	// The ring lookup starts failing, simulating the same instability that's likely to
+	// accompany a ring change in the first place.
+	strategy.setErr(fmt.Errorf("simulated ring lookup failure"))
+
+	// Backdate the pending entry so its grace period has elapsed.
+	backdated := time.Now().Add(-2 * time.Hour)
+	db.pendingNonOwnedRefsMtx.Lock()
+	for r := range db.pendingNonOwnedRefs {
+		db.pendingNonOwnedRefs[r] = backdated
+	}
+	db.pendingNonOwnedRefsMtx.Unlock()
+
+	userBlocksDir := filepath.Join(ingester.cfg.BlocksStorageConfig.TSDB.Dir, userID)
+
+	// Run eviction. The fix's pre-check sees updateTenant fail and skips eviction for this
+	// tenant entirely, rather than consuming the unreconciled queue.
+	ingester.compactBlocksDueToNonOwnedSeries(ctx, 0)
+
+	require.Empty(t, listBlocksInDir(t, userBlocksDir), "no block should be produced while the ring lookup keeps failing")
+	require.Equal(t, uint64(2), db.Head().NumSeries(), "no series should be evicted while the ring lookup keeps failing")
+
+	db.pendingNonOwnedRefsMtx.Lock()
+	require.Len(t, db.pendingNonOwnedRefs, 1, "the pending ref is left untouched, neither reconciled nor evicted, until the ring lookup succeeds again")
+	db.pendingNonOwnedRefsMtx.Unlock()
+}
+
 // TestIngester_compactBlocksDueToNonOwnedSeries_ShouldEvictAgedRefsDespiteFresherOnes verifies
 // that pending non-owned refs are evicted based on their individual grace periods.
 //
@@ -1962,6 +2048,7 @@ type fakeOwnedSeriesRingStrategy struct {
 	mu     sync.Mutex
 	ranges ring.TokenRanges
 	shard  int
+	err    error
 }
 
 func (f *fakeOwnedSeriesRingStrategy) checkRingForChanges() (bool, error) { return true, nil }
@@ -1975,6 +2062,9 @@ func (f *fakeOwnedSeriesRingStrategy) shardSizeForUser(_ string) int {
 func (f *fakeOwnedSeriesRingStrategy) tokenRangesForUser(_ string, _ int) (ring.TokenRanges, error) {
 	f.mu.Lock()
 	defer f.mu.Unlock()
+	if f.err != nil {
+		return nil, f.err
+	}
 	return f.ranges, nil
 }
 
@@ -1988,6 +2078,14 @@ func (f *fakeOwnedSeriesRingStrategy) setRanges(ranges ring.TokenRanges) {
 	f.ranges = ranges
 }
 
+// setErr makes tokenRangesForUser fail with err until cleared with setErr(nil), simulating a
+// ring lookup failure.
+func (f *fakeOwnedSeriesRingStrategy) setErr(err error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	f.err = err
+}
+
 // installFakeOwnedSeriesRingStrategy stops the ingester's real owned-series service, which would
 // otherwise keep ticking in the background and race whatever the test sets up, and replaces it
 // with one backed by a fakeOwnedSeriesRingStrategy reporting initialRanges. The replacement is
diff --git a/pkg/ingester/owned_series.go b/pkg/ingester/owned_series.go
index 7a546716fee..ca8953e2936 100644
--- a/pkg/ingester/owned_series.go
+++ b/pkg/ingester/owned_series.go
@@ -170,7 +170,10 @@ func (oss *ownedSeriesService) updateAllTenants(ctx context.Context, ringChanged
 			continue
 		}
 
-		if oss.updateTenant(userID, db, ringChanged) {
+		// The ring-lookup error, if any, was already logged inside updateTenant; the periodic
+		// ticker just needs to know whether a recompute happened, and a failed lookup already
+		// schedules its own retry on the next tick.
+		if updated, _ := oss.updateTenant(userID, db, ringChanged); updated {
 			updatedUsers++
 		}
 	}
@@ -185,6 +188,9 @@ func (oss *ownedSeriesService) updateAllTenants(ctx context.Context, ringChanged
 }
 
 // Updates token ranges and recomputes owned series for user, if necessary. If recomputation happened, true is returned.
+// err is non-nil only when the ring lookup itself failed, in which case token ranges and
+// pendingNonOwnedRefs were NOT reconciled -- callers that need that reconciliation to have
+// actually happened (as opposed to merely being retried on a future call) must check it.
 //
 // This method is complicated, because it takes many possible scenarios into consideration:
 // 1. Ring changed
@@ -196,7 +202,7 @@ func (oss *ownedSeriesService) updateAllTenants(ctx context.Context, ringChanged
 //
 // Ring and shard size changes require new check of the ring to see if token ranges for this ingester have changed. We also need to check ring if previous ring check has failed.
 // When doing computation of owned series, we make sure to pass up-to-date number of shards.
-func (oss *ownedSeriesService) updateTenant(userID string, db *userTSDB, ringChanged bool) bool {
+func (oss *ownedSeriesService) updateTenant(userID string, db *userTSDB, ringChanged bool) (updated bool, err error) {
 	shardSize := oss.ringStrategy.shardSizeForUser(userID)
 	localLimit := oss.getLocalSeriesLimit(userID, 0)
 
@@ -215,22 +221,22 @@ func (oss *ownedSeriesService) updateTenant(userID string, db *userTSDB, ringCha
 
 	if !ringChanged && reason == "" {
 		// Nothing to do for this tenant.
-		return false
+		return false, nil
 	}
 
 	// We need to check for tokens even if ringChanged is false, because previous ring check may have failed.
 	// If this ingester doesn't own the tenant anymore, ringStrategy is expected to return nil ranges. In that case there will be no "owned" series.
-	ranges, err := oss.ringStrategy.tokenRangesForUser(userID, shardSize)
-	if err != nil {
+	ranges, tokenRangesErr := oss.ringStrategy.tokenRangesForUser(userID, shardSize)
+	if tokenRangesErr != nil {
 		ownerKey, ownerValue := oss.ringStrategy.ownerKeyAndValue()
-		level.Error(oss.logger).Log("msg", "failed to get token ranges from user's subring", "user", userID, ownerKey, ownerValue, "err", err)
+		level.Error(oss.logger).Log("msg", "failed to get token ranges from user's subring", "user", userID, ownerKey, ownerValue, "err", tokenRangesErr)
 
 		// If we failed to get token ranges, set the new reason, to make sure we do the check in next iteration.
 		if reason == "" {
 			reason = recomputeOwnedSeriesReasonGetTokenRangesFailed
 		}
 		db.triggerRecomputeOwnedSeries(reason)
-		return false
+		return false, tokenRangesErr
 	}
 
 	if db.updateTokenRanges(ranges) && reason == "" {
@@ -241,9 +247,9 @@ func (oss *ownedSeriesService) updateTenant(userID string, db *userTSDB, ringCha
 		if !db.recomputeOwnedSeries(shardSize, reason, oss.logger) {
 			db.triggerRecomputeOwnedSeries(reason)
 		}
-		return true
+		return true, nil
 	}
-	return false
+	return false, nil
 }
 
 func secondaryTSDBHashFunctionForUser(userID string) func(labels.Labels) uint32 {