rmetzger commented on PR #6: URL: https://github.com/apache/flink-connector-redis-streams/pull/6#issuecomment-4398211988
Here's what Claude says about the XREADGROUP vs XREAD approach. Curious to hear your opinion: ## The two consumption models in Redis Streams Redis Streams supports two read commands: | Command | Server-side state | At-least-once mechanism | | ------------ | ------------------------------------------------- | ----------------------------------------------------------------------- | | `XREAD` | None — you supply the offset every call | Track offset in client state (Flink checkpoint) | | `XREADGROUP` | Per-consumer-group **PEL** (Pending Entries List) | Server tracks delivered-but-unacked entries; XACK removes them from PEL | `XREAD` is the closer analog to Kafka: stateless from the broker's perspective, the consumer owns the offset, and durability comes from retention. `XREADGROUP` has no Kafka analog — it's more like RabbitMQ / SQS where the broker holds an "in-flight" set per consumer until you ack. ## What this connector chose Grep result is unambiguous: **all reads use `XREADGROUP`** (lines 409, 458 in `RedisStreamsSplitReader.java`). There are 4 hits for `xreadgroup` and 0 for plain `xread`. `consumerGroup` is a required config value (with a default `"flink-redis-streams-default"`). So **there is no XREAD / no-consumer-group mode** in this implementation. Every read goes through a consumer group, every entry lands in the PEL, and every entry must eventually be XACKed — which is the entire reason for the deferred-ACK lifecycle, PEL recovery, and ~half the complexity of the 1048-line SplitReader. ## Why this is a real design choice (and arguably the wrong one for Flink) The author's choice locks the connector into the XREADGROUP model. Trade-offs: **Arguments for XREADGROUP (what the PR implicitly assumes):** - Server tracks PEL: even if Flink state is lost, the broker remembers what was delivered. - Consumer-group lag visibility via `XINFO GROUPS` / `XPENDING` (operator-friendly). - Multiple Flink jobs (or external consumers) can share a group and load-balance. - Idiomatic for Redis-native deployments where teams already use consumer groups. **Arguments for XREAD (the Kafka-style alternative):** - Vastly simpler connector: no XGROUP CREATE, no XACK, no PEL recovery, no deferred-ACK queue, no circuit-breaker-on-XACK-failure, no per-checkpoint snapshot. Probably the SplitReader becomes ~300 lines instead of 1048. - Offsets in Flink state are the single source of truth — same model the Flink Kafka connector uses (and the model that gives you exactly-once via aligned snapshots). - No consumer-group-name footguns (UUID per redeploy, parallelism-rescaling orphans PEL entries — both are documented limitations of the current design). - Many Redis deployments don't have time-based retention configured, so XREADGROUP's "PEL preserves data" advantage is overstated — if your stream is unbounded, retention is your problem either way; if your stream is `MAXLEN`-trimmed, neither model recovers a trimmed entry. ## What I'd push back on the author about This is worth raising on the PR. Two things to ask: 1. **Why XREADGROUP and not XREAD?** The PR description doesn't justify the choice — it just describes the deferred-ACK lifecycle as a fait accompli. The complexity budget for XREADGROUP is large, and many of the issues my reviewers found (NOGROUP escalation, BUSYGROUP race, PEL recovery null handling, XACK retry strategy, default-UUID consumer-name footgun, parallelism-orphans-PEL caveat) are *intrinsic to using consumer groups*. They all disappear with XREAD. 2. **Should there be an `XREAD` mode as a config option?** Even if XREADGROUP stays as the default, an `XREAD` mode would (a) give users an "I just want Kafka-style consumption" path, (b) be the natural choice for users who already partition their streams at the application level (which is exactly what this connector forces via one-split-per-key — the consumer-group benefit is largely defeated already), and (c) provide a simpler-to-debug fallback when consumer-group config goes wrong. The Flink Kafka connector is the precedent here: it stores offsets in Flink state by default and only optionally writes them to `__consumer_offsets` for visibility. That's the same architectural pattern Redis Streams + XREAD would follow. **Short answer to your question:** No, this implementation doesn't offer a no-XACK / no-consumer-group mode. Every read is `XREADGROUP`. The XACK complexity is a downstream consequence of that single design choice, and it's a choice worth questioning in review. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
