mlevkov commented on PR #3798: URL: https://github.com/apache/iggy/pull/3798#issuecomment-5364977008
Rebased onto master and followed up on `af9ce9548` (#3855, source batch acknowledgments), which landed after this PR was opened and changes what this connector owes the runtime. `on_batch_result` has a default no-op in the `Source` trait, so this compiled and stayed quiet, but the default is documented as suitable only for sources with no staged or destructive work. `poll()` here drains the crossfire bridge destructively, so a NACK had nothing to act on: the events existed nowhere else and were simply gone. The connector now holds each batch until the runtime acks it and replays it on the next `poll()` otherwise, which is what lets it recover a failed send instead of absorbing it. Nothing is ever abandoned. Answering 200 already told the sender this gateway owns the event, and the only honest way to shed load is the 429 the handlers return once the bridge fills, which senders retry. Dropping a staged batch would trade that bounded, visible backpressure for silent loss that grows with the length of the outage. The usual case for a give-up bound does not apply here either: oversized bodies are rejected with 413 before a handler runs, headers are clamped on accept, and `Schema::Raw` cannot fail to decode, so a permanently undeliverable batch is not reachable from the accept path. Two existing poll tests were asserting the pre-ack behaviour by polling twice without acknowledging, and now ack between the two. Four new tests cover replay on NACK, release on ACK, an empty state-only batch not staging a replay, and the never-abandon property itself so it cannot be quietly reversed. **One thing that needs a decision outside this PR: #3941.** The SDK stops a source after five consecutive NACKs, which is about 1.5s of backoff plus five send rounds, so roughly two seconds of broker unavailability ends the poll task. That is correct for a source that can re-read its cursor and wrong for this one, whose bridge is in memory: the listener keeps accepting, the bridge fills to `buffer_capacity`, and a restart loses up to 10,000 events that already received a 200. The stop is also unobservable, since the runtime's forwarding loop stays parked and `iggy_connectors_sources_running` keeps counting the source. There is a loophole that would let a plugin survive this (an empty batch always acks, which resets the breaker's counter) and I have deliberately not used it, because it defeats an SDK safety mechanism from inside a plugin. The README states the residual window and points at #3941 rather than working around it here. `README.md`'s producer-failure loss window was also stale: it said there is no feedback channel back to the handler, which #3855 made false. -- 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]
