Skip to content

Separate batch acceptance from Redis transport health #105

Open

Description

@derekste

PR #91 is conflicting and combines several independent memory, threading, API, and reconnect changes. The remaining batch-path problem should be implemented on current main after #104 rather than by reviving that branch.

Problem

The batch APIs can infer connection health from the number of accepted stream writes. An all-server-rejected batch may spuriously enter reconnect even though Redis is healthy, while a mixed accepted/rejected batch can hide the transport state.

Required change

  • Represent transport/connection failure separately from per-item Redis acceptance.
  • Do not reconnect for server-side ReplyError or for an all-rejected explicit-ID batch.
  • Preserve and document the partial-result contract for mixed accepted/rejected batches.
  • Follow the agreed redis-plus-plus reconnect policy for genuine connection loss.

Acceptance

  • One rejected add leaves the existing connection usable.
  • An all-rejected batch does not reconnect.
  • A mixed batch returns the documented partial result without claiming transport failure.
  • A genuine disconnect follows the intended reconnect/retry policy.
  • Implement as a focused PR on current main; do not merge Fix thread safety, memory bugs, and error handling #91 wholesale.

Activity

  1. derekste commented on Sep 29, 2026

    @derekste
    MemberAuthor

    Reproduced at 0bb9c35e7480e5d01afd58a5ed5ce27f3191f226 on private Redis: a single duplicate ID returned RA_NOT_CONNECTED and caused two new connections; an all-rejected duplicate/backward batch also caused two connections. The caller's connection was healthy in both cases.

    Fix PR: #109, stacked after #108. Native macOS lifecycle/write/fault tests pass (3/3). They cover accepted/rejected mixed results across all batch APIs, genuine socket loss after successful Redis acceptance, recovery even when another batch item succeeded, and recorded-command evidence that nothing is replayed. Full Linux validation is running.

    Keep this issue open until the fix merges and passes validation. Related IOC investigation: fermi-ad/redis-pvxs-ioc#24; neither this reproduction nor the cursor fixes establish the cause of the historical #73 allocator crash.

  2. bigsamich commented on Sep 30, 2026

    @bigsamich
    Contributor

    Verified against #109 (904e0ef) and the stack head. All four behavioural criteria hold:

    • A single rejected add returns RA_REJECTED with 0 new connections, and the connection stays usable. On main it returns RA_NOT_CONNECTED and opens 2 new connections.
    • An all-rejected batch doesn't reconnect.
    • A mixed batch returns only the accepted IDs, without claiming a transport failure.
    • A genuine CLIENT KILL returns RA_NOT_CONNECTED, reconnects, and replays nothing.

    Two notes:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions