lianetm commented on PR #15640: URL: https://github.com/apache/kafka/pull/15640#issuecomment-2115938603
High level comment, just to clarify and make sure it's something we are considering and will cover with the follow-up PRs for timeouts. Here we're introducing a component to ensures that app events are expired only after having one chance, but that's only at the app thread level, and not for all events, but only for unsubscribe, and poll. Thing is that events can also be expired indirectly when a request is expired (so playing against this changes). So even if the `processBackgroundEvents` introduced here gives an expired event a change to run one, that won't actually happen (because the underlying request expires). I expect that side needed to make this whole intention work in practice will be a follow-up PR, am I right? Also almost none of our integration test cover the poll(ZERO) case (helper funcs [here](https://github.com/apache/kafka/blob/056d232f4e28bf8e67e00f8ed2c103fdb0f3b78e/core/src/test/scala/unit/kafka/utils/TestUtils.scala#L892-L910), used by most of the test, poll with timeout > 0). That's probably why we did not find the expiration issues we have with poll(0) before. I guess that after addressing the timeout/expiration (this PR and follow-up), we should be able to add some. -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
