Pankraz76 commented on PR #22680: URL: https://github.com/apache/kafka/pull/22680#issuecomment-6463442734
Follow-up on points 2 and 3 from my comment above: opened #23993 under KAFKA-20734, since it fixes a regression from this PR before it ships. **Busy loop** - Retrying parked markers from `generateRequests()` no longer calls `wakeup()`. That path already runs on the sender thread, and markers that get a leader are drained to their broker queue in the same call, so the wakeup only made the next `poll` return immediately and re-park the marker. - While markers sit in `markersQueueForUnknownBroker`, `doWork()` polls with a 100 ms bound instead of `Long.MaxValue`. They are retried after a backoff, not in a tight loop and not only when some unrelated event wakes the thread. It's the same 100 ms backoff `NodeToControllerRequestThread` uses when no controller is known. - `addTxnMarkersToBrokerQueue` from other threads and from completion handlers still wakes up the sender, so nothing changes there. - I used a fixed 100 ms rather than `retry.backoff.ms` or a metadata-update hook to keep the change small. Happy to switch if you'd prefer one of those. **Tests** - `TransactionMarkerChannelManagerTest`: re-parking a marker does not call `wakeup()`, and `doWork()` uses the bounded poll only while markers are parked. Both tests fail on trunk and pass with the change. - `TransactionsTest` (end-to-end, from #23947): commit while a partition hosted off the coordinator is offline, restart the broker, and check that a `read_committed` consumer sees the committed record. If KAFKA-20734 gets cherry-picked to 4.3 / 4.4, #23993 should go along with it. cc @jolshan @kevin-wu24 -- 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]
