Recover owned readers across observed stream resets

[derekste]

Recover rejected stream reads without making metadata permission a prerequisite for delivery. A nonblocking XREAD check quarantines only failing keys, then periodically rechecks them. Healthy neighbours continue when XINFO is denied, and legacy registrations resume after an owned inspecting peer is removed. This PR depends on #109 and the revised #108/#127 foundation.

Continuity inspection now defaults off, with a per-subscription override. A persistent deadline queue batches at most 16 due XINFO requests, skips active keys, backs off idle keys and denied inspection, and uses finite/adaptive read cycles. Unsupported Cluster inspection reports unknown evidence and does not suppress XREAD. The default read path avoids cursor-map copies and repeated all-registration success walks.

Reader status distinguishes idle replies, socket failures, key rejections, inspection failures and retention gaps. Unresolved status cursors are empty, inactive handles clear health flags, and queued older-epoch data is fenced. subscribeStreamWithEpoch() passes the batch's captured fencing token directly. Observed cursors and callback-delivery counters remain distinct. subscribeStreamWithMetadata() additionally provides immutable epoch/rejection evidence captured when the reader admits each batch; the existing epoch callback API wraps it without changing callers. A queued valid callback cannot acknowledge a later read rejection.

New batch-evidence validation: all 34 native standalone recovery tests pass, including a held callback across real ACL XREAD rejection with one and four workers. IOC scalar and NTNDArray queued-batch recovery regressions pass; the complete integrated native IOC suite passes 25 tests with this exact adapter revision. The current-head adapter build check and successor #128 ASan/UBSan and TSan checks pass. IOC source-health #128 also passes all four implementation-head platform/sanitizer checks and retains the Linux idle-probe comparison; its enabled default still needs owner acceptance.

Earlier foundation validation: all 103 tests in the Linux Release/C++20/Redis 7.4.2 stack passed. Four additional retained-history/authentication cases passed natively and under ASan+UBSan. The full applicable macOS sanitizer suite passed 100 cases. A private three-master Redis 7.4.2 Cluster delivered all six owned and all six legacy updates with inspection enabled; that fixture is now included in CI. Tests use unique keys/users and terminate only the fixture owner's clients.

[derekste]

Follow-up review, 30 September 2026: hold this PR for the reproduced ACL-transition defect in #113 and a decision on the default probe policy.

Recovery from observed stream replacement and isolation of wrong-type keys are useful. The current implementation can permanently suppress a repaired, readable key after XINFO permission is revoked. The earlier XINFO-denied test starts without a cached exclusion and does not cover this transition.

The default one-second inspection also has measurable payload traffic. With an idle stream retaining two 1 MiB entries, Redis sent 4,196,382 bytes over 2.2 seconds with readerProbeMs=1000, versus 1,421 bytes with probes disabled. Measurement started after initial delivery; output-byte totals include the small control replies. XINFO returns both first/last payloads, and inspection runs before XREAD in the reader loop.

Recommendation: make inspection an explicit adapter opt-in and enable it deliberately in the IOC after qualification, or provide qualification that justifies the enabled default. The existing IOC capacity report used adapter 0bb9c35 and does not cover this overhead. This is a policy/qualification concern distinct from the reproduced bug.

The parent lifetime blocker in #108/#112 also remains. Keep Cluster cleanup deferred, retain source timestamp/cursor contracts, and obtain independent Instrumentation approval after fixes and fresh validation.

[bigsamich]

Reviewed at 0c3b0e6, in the same isolated setup as #108/#109. I agree with holding for #113 and for the probe-default decision; I've added detail on both in the existing threads. The core mechanism is well built. The problems are in the edges around it, and a few are as serious as #113.

What works (verified)

• The epoch fence is simple and correct. Delivery is exactly-once and in order per epoch: 0 invariant violations across 14 stress runs covering DEL/recreate/wrong-type churn, CLIENT KILL outages, handle move/reset churn and concurrent status() polling, including TSan runs. Queued but undelivered data is neither lost nor duplicated when a second registration or a reset rewinds the key.
• Reset bookkeeping is exact for every documented case: deletion, key expiry, FLUSHALL, XDEL of the newest entry, trim-to-empty, XSETID below the cursor, and a restart without persistence. Exact cursors survive ordinary outages with no replay. Denied XINFO degrades exactly as documented, including when it is revoked mid-run.
• The regression claim is exact. A reduced copy of the delete/recreate test fails on 904e0ef (5/5) and passes here (8/8).
• The recovery test is stable on TCP: 414/415 runs across plain, ASan and TSan at 0.5-2 CPUs, with 0 sanitizer reports in 220 sanitizer runs. XINFO parsing is defensive: fields are looked up by name, a nil first-entry is handled, and no payload is copied. readerProbeMs=0 really is free on the wire.

Blocking (in addition to #113)

1. Redis Cluster. With the default readerProbeMs, every bucket that has an owned subscription never issues XREAD again. Owned subscriptions and legacy readers under the same base key deliver nothing, silently (RedisConnection.hpp:326). This is a regression from #108/#109/68e01c7, and the fix is 4 lines. If no deployment can ever reach a cluster-enabled server, this drops to "should fix", but then it should fail loudly.
2. A second trigger for the #113 class that needs no ACL change. Resetting an owned handle while its key is the wrong type means a legacy reader on that key never recovers. A fix placed inside the XINFO-rejected branch can't reach it (RedisAdapter.cpp:452, and the #113 thread).

Should fix

3. After a rejected read, a forced XINFO sweep runs every 50 ms: about 20× the configured rate, and 40 MB/s per 1 MiB-payload key (RedisAdapter.cpp:634).
4. connected/socketTimeouts/reconnects are wrong on idle streams when probing is off or denied, which is the proposed opt-in default (RedisAdapter.cpp:489; the root cause is pre-existing, #122).
5. Scaling:
   • an O(K²) walk per interval (RedisAdapter.cpp:548);
   • a per-iteration map copy and registration walk, even with probing off (RedisAdapter.cpp:625);
   • the rotation reset and the 64-key cap limit coverage (RedisAdapter.cpp:615);
   • serial XINFO round trips in front of XREAD (probe thread).
6. $ subscriptions on a wrong-type key lose post-repair entries (RedisAdapter.cpp:529), and cxn.timeout=0 disables detection entirely (RedisAdapter.cpp:630).
7. recovery_test.cpp kills every client of the Redis it targets (recovery_test.cpp:87).

Minor / nits

• Failure accounting and the line-88 flake (RedisAdapter.cpp:563).
• Leftover keys and PID reuse in the test (recovery_test.cpp:48).
• retentionGaps misses a certain loss (RedisAdapter.cpp:539).
• Substring error matching (RedisConnection.hpp:351).
• ReaderStatus shape and epoch attribution (RedisAdapter.hpp:147).
• The isolation scope isn't stated in the docs (docs/stream-subscriptions.md:64).
• A reset replays the whole retained history in one batch, because of the count-less XREAD (see the COUNT fix on #108). With 100 × 1 MiB retained, that is one 100-entry batch and +294 MiB peak RSS. After a restore from an older snapshot, every replayed entry is a duplicate.
• Readability: the connection-state transitions are coded three times (reader_result, and per key and bucket-wide in inspect_readers), and 15 of the 221 added lines hold two or more statements. One observe(registration, Observation) helper and one statement per line would make the state machine reviewable.

Docs / CHANGELOG

There is no CHANGELOG entry for ReaderStatus, status() or readerProbeMs, nor for the default-on per-key XINFO probing (with its traffic and ACL requirements), wrong-type exclusion, or epoch rewinds that can deliver IDs below the previous maximum. docs/api.md and README.md still advertise Cluster.

Issues

• Fixes #110 holds for owned readers with default options. Verified: the recreated 1-0 is delivered after about 1 s with streamResets=1 epoch=2, and an outage preserves cursors with exactly-once delivery. Caveats worth adding to #110:

  • legacy readers still stall (documented);
  • readerProbeMs=0 disables recovery;
  • with cxn.timeout=0, detection never runs;
  • on Cluster, nothing is read at all.

  As with #109, the keyword only fires if this PR targets main when it merges.

• Add Fixes #113 with the fix.

• Reference #93 as partially addressed (owned keys only; see the docs comment) and #65 (Cluster behaviour).

• CI: there are no automatic checks for this base branch. Until the PR is retargeted, the linked manual run (18/18 at this head) is the only evidence.

[derekste]

@bigsamich Thanks for the recovery stress tests and the additional boundary cases. I'm keeping this PR blocked for #113 and a revised probe policy, including the Cluster read-suppression finding.

I propose moving wrong-type isolation to the read path: quarantine keys that actually reject a non-blocking XREAD, periodically recheck those keys, and resume them after repair. That must preserve healthy neighbors when XINFO is denied and handle an owned subscription being removed while a legacy reader remains. Resetting every cached kind to Unknown on XINFO rejection would break that isolation.

Inspection being unsupported must not become a transport failure or suppress XREAD. That needs a narrow regression fix here; broad Cluster removal remains #65. I'll also separate idle blocking-read expiration from disconnection, bound detection when cxn.timeout=0, and correct the $ cursor, failure/epoch, and empty-retention accounting cases.

For probing, I propose an explicit per-subscription opt-in with an adapter default, bounded scheduling and NOPERM backoff. The rejected-read path must not bypass the configured interval. The O(K²) walk, repeated cursor copies, rotation starvation, and serial round trips need correction or qualification before we describe the overhead as acceptable. Imaging and large-key-count measurements must include the enabled policy.

The recovery tests need a private Redis instance or targeted client termination, unique keys, and cleanup on failure. I'll update the ReaderStatus/API and compatibility documentation, cover the remaining review points, and request fresh CI and review on the retargeted head. #110 and #113 remain open until the applicable fixes reach main.

The fixes above are in progress. I'll post the changed commits and validation before requesting another code review.

[derekste]

@bigsamich, the recovery revisions are pushed and ready for another pass.

Wrong-type isolation now follows the read path using nonblocking XREAD $ checks. The four requested acceptance cases pass at one and four workers: repair after XINFO revoke, still-invalid isolation after revoke, a surviving legacy reader after its owned inspecting peer is reset, and a legacy wrong-type key beside a healthy owned reader. A cached metadata kind no longer controls permanent exclusion. Cluster inspection is explicitly unsupported evidence while owned and legacy reads keep flowing.

Inspection defaults to zero, with per-subscription selection. Due probes use a persistent deadline queue and bounded 16-command pipeline, transfer one retained payload, skip active keys, and back off idle/denied inspection. Rejected reads do not trigger repeated forced metadata sweeps. Cursor maps are reused without exclusions, and successful read-state walks occur only on a state change. Read cycles shorten for checks and remain finite with timeout zero.

Status now separates idle NIL from a real socket timeout, counts inspection transport failure across the bucket, attributes key rejections to failing registrations, and detects empty retained-history loss. Prefix matching prevents a username containing WRONGTYPE from becoming an invalid-stream verdict. Epoch zero means no registration; unresolved status cursors are empty; inactive handles clear health flags. Captured-epoch callbacks and queued-epoch fencing distinguish observed data from callback handoff. The changelog and API/build guides describe the costs and limits.

Evidence: 103/103 full Linux Release/C++20/Redis 7.4.2 cases passed; four added retained-history/authentication cases passed natively and under ASan+UBSan; 100 full applicable macOS sanitizer cases passed. A private three-master Cluster delivered 6/6 owned and 6/6 legacy updates with probing enabled. Test keys/users are unique and cleaned up; client termination targets only the test owner. CI now includes the private Cluster case.