[danielblando]

What this PR does:

Stops the Ruler retrying staleness markers that can never be accepted.

The failure case

When a recording rule is moved from one rule group to another within the same namespace, the two groups race over the rule's output series:

1. Group A reloads and no longer contains rule R. Every series R produced is queued into group A's staleSeries list.
2. Group B reloads with R and immediately starts writing those same series.
3. At the end of every evaluation, group A's cleanupStaleSeries appends a staleness marker for series that group B keeps advancing. The append is rejected and the push returns 400.
4. cleanupStaleSeries clears staleSeries only when Commit() succeeds, so the whole list is re-appended on the next evaluation, and the next, until the ruler process restarts.

Each retry uses the current evaluation timestamp, so it is not replaying an old sample: it re-attempts the same series set against series another group keeps advancing faster. It can never succeed. Group.CopyState also copies staleSeries to the successor group, so reloading the namespace does not reset it.

Why Cortex hits this and Prometheus does not

Prometheus' own appenders return these errors synchronously from Append, so the per-sample switch in cleanupStaleSeries consumes them, the rejected sample is never staged, Commit() succeeds and the list is cleared:

case errors.Is(unwrappedErr, storage.ErrOutOfOrderSample),
	errors.Is(unwrappedErr, storage.ErrTooOldSample),
	errors.Is(unwrappedErr, storage.ErrDuplicateSampleForTimestamp):
	// Do not count these in logging, as this is expected if series
	// is exposed from a different rule.

PusherAppender batches: Append buffers and returns nil, the batch is sent in Commit(), and the rejection surfaces there instead. The storage.Appender contract says a non-nil Commit means the transaction was rolled back, so a caller is entitled to read it as "nothing was written, retry". For a 4xx that is not what happened — the rejected samples will never be accepted, and the rest of the batch was written.

The fix

PusherAppender.Commit() reports success for a 4xx when the transaction consists only of staleness markers, so the caller drops them instead of retrying forever. 5xx and non-HTTP errors are unchanged: those are genuine failures where retrying is correct.

A transaction carrying rule results keeps returning the error, because the Ruler surfaces it as rule health (SetHealth(HealthBad), SetLastError, EvalFailures) rather than retrying it. Swallowing it there would hide recording-rule output that is not landing. cleanupStaleSeries is the ruler's only write path that retries at all — rule results and in-evaluation markers are recomputed each evaluation at a new timestamp, so they are bounded by construction.

Dropping a marker costs at most one lookback window of a retired series lingering in query results. In this race the marker is wrong anyway, since the series is still being written by the other group, and the list is already discarded silently on every ruler restart — which is how these episodes end today.

Which issue(s) this PR fixes:

No existing issue — the behaviour is described above.

Checklist

• Tests updated
• Documentation added — n/a. No user-facing config, API or metric change.
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
• docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags — n/a, no new flags.

[SungJin1212]

Should 429 be handled differently here? Unlike a 400 from out-of-order or duplicate samples, a 429 from the ingestion rate limiter rejects the whole request before anything is written in the Distributor layer.

Couldn't this leave stale markers unwritten because of a 429?

What about if onlyStaleMarkers && resp.Code != http.StatusTooManyRequests?

[danielblando]

Makes sense @SungJin1212. Good catch.