[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-855921781 Thanks for the reviews @hachikuji @guozhangwang. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-843148293 @hachikuji I fixed the tests and made you suggested changes, but what do you think about @guozhangwang's point

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-829027535 @hachikuji that's much better, thank you. I added some logging where we ignore on the remove path, hope that's OK. -- This is an automated message from the Apache Git

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-828333212 @hachikuji Thanks, I was being slow on the uptake, but what you describe makes perfect sense. I've moved the logic to the GroupMetadataManager, am using the max observed epoch

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-824800741 Thanks @hachikuji, it's much simpler now, if you could take another look? -- This is an automated message from the Apache Git Service. To respond to the message, please log

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-822355393 @hachikuji I've addressed your comments, if you want to take another look? -- This is an automated message from the Apache Git Service. To respond to the message, please log

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-784956360 Rebased for conflict. Grateful if you could take a look @hachikuji. This is an automated message from the

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-769621007 Thanks, and no worries about the wait. This is an automated message from the Apache Git Service. To respond to

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-766880511 Rebased for conflict. @guozhangwang @cadonna @hachikuji please could one of you take a look? This is

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-766880511 Rebased for conflict. @guozhangwang @cadonna @hachikuji please could one of you take a look? This is

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-745239513 @guozhangwang that's a much better solution, thanks! I've implemented that and rebased for a conflict. This

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-738925531 You're right @guozhangwang, `ScheduledFutureTask` contains a sequence number to break ties, so the executor _is_ FIFO. So you're saying that the wrong network thread handling

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-713663078 @guozhangwang, @hachikuji I was thinking... solving it as I have now (keeping track of the epoch) doesn't address another potential problem case where the tenure as leader is

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-710044807 > One thing I didn't understand was why I needed to change `AbstractFetcherManagerTest`. Stupid me, that was because I turned up the logging. I'll revert that.

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-710030951 @hachikuji I'm now passing in the coordinator epoch and ignoring attempts to unload state if the epoch is old than what's loaded. I've also added a test. One thing I didn't

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-709457342 I assumed the scheduler would be using a heap to prioritize scheduled tasks, and that wouldn't preserve the order of submitted tasks with the same scheduled time. Since

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-709439774 @hachikuji, yes I see that although replica manager might be synchronized it doesn't prevent the race. I was already working on introducing the epoch into the group

[GitHub] [kafka] tombentley commented on pull request #9441: KAFKA-10614: Ensure group state (un)load is executed in the submitted order

tombentley commented on pull request #9441: URL: https://github.com/apache/kafka/pull/9441#issuecomment-709225396 @guozhangwang this is a draft implementation of one of the solutions you suggested. I will try to add a test, but in the meantime any comments you have are welcome.