numinnex commented on PR #4205:
URL: https://github.com/apache/iggy/pull/4205#issuecomment-5711210700

   A quick primer on the status quo is needed here, because it changes what 
this document should aim at.
   
   We never implemented Kafka's producer id and sequence machinery and do not 
intend to. What we do have, landed and on by default, is deduplication on the 
partition plane keyed on `(client_id, user_id, request)`, where the client id 
is a `u128` the client itself generates. Each partition group holds a 
per-client watermark with a 128-deep committed window below it, sized by 
`[partition] dedup_clients_max` at 4096 per group. Retried produces are what it 
exists to absorb, and the shape is nearer Kafka's than it looks: a producer id 
with a per-partition sequence maps onto a session id with a request id, both 
per partition, and our window is deeper than the five batches a Kafka producer 
keeps in flight.
   
   What breaks the mapping is which hop each side protects. Ours covers gateway 
to Iggy, Kafka's covers producer to gateway. A retrying producer sends a fresh 
Produce request, the gateway turns it into a fresh Iggy send carrying a fresh 
request id, and nothing recognises the replay. Closing that means deriving both 
halves of our key from the Kafka request instead of letting the SDK mint them, 
session id from the producer id and request id from the base sequence. 
`ConsensusSession::with_client_id` is public, but `TcpClient` builds its 
session at `tcp_client.rs:378` and `:583` with no way to pass one in, and 
`next_request_id` has no caller-supplied form, so the SDK needs work before the 
gateway can lean on any of this.
   
   Worth stating plainly what it buys, because it is not the whole contract. 
Retries stop duplicating, but an absorbed duplicate answers empty success with 
no base offset, so the retry reports an unknown one, and the watermark accepts 
anything above it without noticing a gap, which is why never sending 45 or 46 
stays right.
   
   That makes your producer id layout more load-bearing rather than less, since 
the id is what a session key would derive from. Does the counter survive a 
restart? Transactions stay out permanently, and the unsupported-version answer 
is the one to keep.
   
   Four smaller things in the mapping doc: an empty record key has no path, 
since a zero-length key is not a null one and an empty header value is rejected 
at the same line cited for the 255-byte cap. The decompression bound is per 
batch where it needs to be per request, as one frame carries up to the 
4096-element budget's worth of batches. Header order is lost to the `BTreeMap` 
and the doc never says so. OffsetFetch with a null topic list cannot reverse 
`TopicMapping::resolve`.
   


-- 
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]

Reply via email to