gkoszyk commented on PR #4205: URL: https://github.com/apache/iggy/pull/4205#issuecomment-5711053968
Storage direction is right, and the doc is unusually well grounded. I checked the Iggy-side claims against the tree and they hold, including the load-bearing one: a named consumer hashes to a `u32` and can collide, a named consumer group resolves through metadata to a monotonic id. Two substantive things. ## Idempotence: point it at our dedup instead of allocating ids We don't do Kafka-style idempotence and won't. What we do have, landed and on by default, is partition-plane dedup keyed on `(client_id, user_id, request)`, where `client_id` is a `u128` the client generates. The watermark is per consensus group, so per partition, with a 128-deep committed window below it. Sized by `[partition] dedup_clients_max`, 4096 per group, ceiling 65536. It covers retried produces explicitly. That is close to Kafka's key shape. Producer id plus a per-partition sequence maps onto session id plus request id, per partition on both sides, and our window is wider than the 5 batches Kafka's producer keeps. The catch is which hop it protects. Our dedup covers gateway to Iggy. Kafka idempotence covers producer to gateway. A producer retry arrives as a fresh Produce request, the gateway makes a fresh Iggy send with a fresh request id, and nothing absorbs it. To make this count, the gateway has to derive both halves of the key from the Kafka request: session id from the producer id, request id from the base sequence. Two SDK gaps sit in the way. `ConsensusSession::with_client_id` exists and is `pub`, but `TcpClient` hardcodes `ConsensusSession::new()` (`tcp_client.rs:378`, `:583`), so nothing plumbs a chosen id through the builder. And `next_request_id` is an internal monotonic counter with no caller-supplied form. Both live in the same area as the open session-resume work. What the mapping does and does not buy, so the doc can be honest about it: - Duplicate suppression on retry: yes, and that is what `enable.idempotence=true` buys most users. - The original offset back on an absorbed duplicate: no. A dedup hit answers the operation's empty success with no base offset, so the retry surfaces an unknown offset. - Gap detection: no. The watermark accepts anything above it without noticing a hole, so never sending 45 or 46 stays correct. - Fencing: no. Partition slices carry no epoch, so there is no producer-fenced equivalent. - It degrades quietly. At `dedup_clients_max` the oldest client entry is evicted and its next replay re-executes, so the guarantee weakens as producer count per partition grows. Your producer id layout gets more useful under this, not less. The producer id is what a session id would be derived from, and cross-instance uniqueness is what stops two gateways sharing a dedup key. Please also say whether that counter survives a gateway restart, since a counter that resets reissues ids and breaks the same layer the layout exists to protect. Transactions stay out, permanently. The unsupported-version answer for a transactional id and the reasoning about the Java client's fatal set are both right. Keep them. ## Four things in the mapping doc 1. An empty record key has no path. Kafka separates a null key from a zero-length one, the key is stored as a header value, and an empty header value is rejected at `user_headers.rs:631`, the same line cited for the 255-byte cap. The fallback list covers a key that is too long but not an empty one, and the null-and-empty section covers values only. Such a record either fails to store or comes back with a null key. 2. The decompression bound is per batch where it has to be per request. One frame can carry many batches, bounded only by the 4096-element budget in `bounds_guard.rs`, so the real ceiling is that times `max_frame_size` rather than `max_frame_size`. This is the same product-across-nested-arrays shape `MAX_REQUEST_ELEMENTS` was added for, and that module's doc has the history. 3. Header order is not preserved and the doc does not say so. User headers live in a `BTreeMap` ordered by kind then bytes, so Fetch emits them sorted by name rather than in producer order. Duplicates are handled by the envelope, order is not mentioned anywhere. 4. OffsetFetch with a null topic list cannot reverse the topic mapping. `TopicMapping::resolve` is one way and an override can point at a different stream, so enumerating one stream both misses topics and hands back Iggy-side names where Kafka names are required. ## Process #3533 is closed, so the mapping doc has no open issue to land under, and #3540 asks for sign-off on options A, B or C while this proposes a fourth. Worth reopening something or filing a docs issue so the epic checklist tracks these three files. I would drop the 22 September default-if-no-answer clause, since #3540's own acceptance criteria call for maintainer sign-off. -- 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]
