lianetm commented on PR #16885: URL: https://github.com/apache/kafka/pull/16885#issuecomment-2304700659
Hey! I just pushed all the requested changes. The most important change is addressing @AndrewJSchofield 's [comment](https://github.com/apache/kafka/pull/16885#pullrequestreview-2247401944) regarding timeout handling, totally agree. > Couldn't the OffsetsRequestManager simply make the sequence of RPCs to complete the little dance, regardless of a timeout? Yes, I think we could and I went ahead and did it. Basically the `updatePositions` does its sequence of async operations (not blocking the background thread), and ends up actually updating the positions when it gets all the info it needs only if the set of initializing partitions (partitions requiring positions is the same). The only consideration is that errors could occur also, so we need to keep them to be thrown on the next call to update positions if the triggering updatePositionsEvent already expired. Note that this is exactly the same logic already in place for reset/validate positions, for both consumers. Now we just need it also at the updatePositions level given that it moves on and attempts to do what it needs in the background. I'm currently adding more tests, but all existing test (unit and integration) pass, so please take a look and let me know. Thank you both! -- 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
