fix(cache): read the quarantine timestamp back before deleting a group

[cosmin-staicu]

Stacked on #194, which is stacked on #193. Merge in that order; the diff here is the quarantine fix alone.

What is wrong

Quarantine in RedisStreamHealthMaintainer is a wait. A consumer group is quarantined by writing the current instant into a hash field, and deleted once MaintainerQuarantineInterval has passed. CheckEmptyStreamGroupAsync, the no-consumers path, did exactly that.

CheckStreamGroupWithConsumersAsync, the stale-LastDeliveredId path, wrote the same instant into the same field — and then branched on whether the field existed:

var quarantineValue = await Database.HashGetAsync(context.QuarantineKey, groupInfo.Name, CommandFlags.PreferReplica);
if (quarantineValue.HasValue)
{
    // delete now; the recorded instant is never parsed, never compared
}

The timestamp was written and never read back. Presence stood in for elapsed time, and the two are not the same thing:

1. Overlapping passes shrink the wait to zero. LockTakeAsync holds the lock for MaintainerCheckInterval, and the only LockReleaseAsync is in the catch. A pass that finishes normally leaves the lock to expire, and expiry is on the same clock as the timer — so a pass that outruns its interval is still running when the next one starts. The first writes the quarantine record, the second reads it and deletes the group, same cycle. Nothing waited.
2. With no overlap it was still the wrong interval. "Seen again" was the entire test, so the group died on the following cycle — MaintainerCheckInterval, 30 min by default — rather than after the documented MaintainerQuarantineInterval of 1 h.

What changed

Both paths now take the recorded instant from GetQuarantinedAtAsync and judge it with QuarantineElapsed. There is one copy of the rule where there were two, which is what let them drift apart in the first place. The delete log line carries the instant the group was quarantined, because "was in quarantine" no longer distinguishes anything worth logging.

Behaviour change for anyone watching this path: a group with consumers and a stale last-delivered-id now lives roughly one extra cycle before deletion, because it serves the whole quarantine interval instead of a single pass. That is the contract the option documents, and the one the no-consumers path has honoured all along.

The test that was supposed to catch it

Group_with_consumers_and_stale_LastDeliveredId_is_deleted_when_already_quarantined seeded the quarantine field with DateTimeOffset.UtcNow and asserted the group was deleted in that same pass. It pinned the defect in place rather than catching it — its sibling on the no-consumers path backdates past the interval, as it should.

It now backdates too, renamed ..._is_deleted_once_the_quarantine_has_elapsed. The discriminating case is new: Group_with_consumers_and_stale_LastDeliveredId_survives_a_quarantine_that_has_not_elapsed, written first and confirmed red against the old code. The backdated case passes either way, which is how presence-only deletion went unnoticed through review.

Docs

docs/how-to/broadcast.md described only the no-consumers condition for quarantine. It now names both conditions and says the interval runs from the recorded timestamp rather than from the pass that happens to observe it.

Verification

• Solution builds with 0 warnings.
• Full suite against a live Redis: net10.0 1776/1776, net8.0 1755/1755.

Contributor declaration

• I signed off my commits per the DCO (git commit -s).
• I am contributing on behalf of my employer, or in the course of employment / using employer resources.

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation consistently enforces the documented interval and includes focused regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes stale consumer groups being deleted before completing the configured quarantine interval.

Changes:

• Centralizes quarantine timestamp parsing and elapsed-time checks.
• Adds regression coverage for elapsed and active quarantines.
• Updates maintainer documentation and changelog.

File	Description
RedisStreamHealthMaintainer.cs	Enforces the full quarantine interval.
RedisStreamTopicMonitorTests.cs	Covers active and elapsed quarantines.
docs/​how-to/​broadcast.md	Documents both quarantine conditions.
CHANGELOG.md	Records the defect and fix.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.