[GitHub] [kafka] jolshan commented on pull request #13591: KAFKA-14831: Illegal state errors should be fatal in transactional producer

2023-06-29 Thread via GitHub
jolshan commented on PR #13591: URL: https://github.com/apache/kafka/pull/13591#issuecomment-1613597535 Ok -- test failures seem to be consistent with current trunk and not due to this change. Will go ahead and merge. -- This is an automated message from the Apache Git Service. To

[GitHub] [kafka] jolshan commented on pull request #13591: KAFKA-14831: Illegal state errors should be fatal in transactional producer

2023-06-28 Thread via GitHub
jolshan commented on PR #13591: URL: https://github.com/apache/kafka/pull/13591#issuecomment-1611816013 Some new test failures on latest build. I will rebuild and investigate to see if this is new to trunk. -- This is an automated message from the Apache Git Service. To respond to the

[GitHub] [kafka] jolshan commented on pull request #13591: KAFKA-14831: Illegal state errors should be fatal in transactional producer

2023-06-13 Thread via GitHub
jolshan commented on PR #13591: URL: https://github.com/apache/kafka/pull/13591#issuecomment-1590069645 Thanks @kirktrue, this looks reasonable. Let's fix the conflict and let tests run one more time. -- This is an automated message from the Apache Git Service. To respond to the message,

[GitHub] [kafka] jolshan commented on pull request #13591: KAFKA-14831: Illegal state errors should be fatal in transactional producer

2023-06-07 Thread via GitHub
jolshan commented on PR #13591: URL: https://github.com/apache/kafka/pull/13591#issuecomment-1581564571 > No, the test method would need to specifically mark its thread as wanting to poison the thread or not before running the rest of the test. In the TransactionManagerTest, there was only

[GitHub] [kafka] jolshan commented on pull request #13591: KAFKA-14831: Illegal state errors should be fatal in transactional producer

2023-06-07 Thread via GitHub
jolshan commented on PR #13591: URL: https://github.com/apache/kafka/pull/13591#issuecomment-1581443881 ThreadLocal variables are not as frequently used in Kafka, but I can see a potential argument for usage here. One tricky part about thread locals is testing. Any ideas on how we could do

[GitHub] [kafka] jolshan commented on pull request #13591: KAFKA-14831: Illegal state errors should be fatal in transactional producer

2023-05-05 Thread via GitHub
jolshan commented on PR #13591: URL: https://github.com/apache/kafka/pull/13591#issuecomment-1536545551 https://github.com/apache/kafka/commit/ea81e99e5980c807414651034a8c60426a158ca4 added TransitionToUnitialized. Can we confirm this has the behavior we expect? I believe this would count

[GitHub] [kafka] jolshan commented on pull request #13591: KAFKA-14831: Illegal state errors should be fatal in transactional producer

2023-05-02 Thread via GitHub
jolshan commented on PR #13591: URL: https://github.com/apache/kafka/pull/13591#issuecomment-1532288164 Some small comments, but I think we are very close to merging :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use

[GitHub] [kafka] jolshan commented on pull request #13591: KAFKA-14831: Illegal state errors should be fatal in transactional producer

2023-04-28 Thread via GitHub
jolshan commented on PR #13591: URL: https://github.com/apache/kafka/pull/13591#issuecomment-1528256534 @kirktrue do we have a list of transitions we consider internal vs external? It would be nice to review that list as well as the code. -- This is an automated message from the Apache