[GitHub] [kafka] lucasbru commented on pull request #13876: KAFKA-10733: Clean up producer exceptions

2023-07-11 Thread via GitHub


lucasbru commented on PR #13876:
URL: https://github.com/apache/kafka/pull/13876#issuecomment-1630247754

   @jolshan -- I will discuss the remaining scope with @guozhangwang now that 
he's back. Can we merge this restricted PR?


-- 
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



[GitHub] [kafka] lucasbru commented on pull request #13876: KAFKA-10733: Clean up producer exceptions

2023-06-30 Thread via GitHub


lucasbru commented on PR #13876:
URL: https://github.com/apache/kafka/pull/13876#issuecomment-1614928567

   @jolshan This makes sense. It seems to me that I need to go through the 
whole KIP and have an end-to-end solution before addressing these kinds of 
consistency problems. At least for the fencing exceptions, I'm leaning towards 
leaving them unwrapped now (and all other fatal exceptions). But I'm not super 
confident, because it seems Guozhang explicitly decided against that solution. 
Now, to make some progress here I would say I remove the wrapping changes to 
`ProducerFencedException` and `InvalidProducerEpochException` and we merge the 
other, less intrusive changes?


-- 
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



[GitHub] [kafka] lucasbru commented on pull request #13876: KAFKA-10733: Clean up producer exceptions

2023-06-28 Thread via GitHub


lucasbru commented on PR #13876:
URL: https://github.com/apache/kafka/pull/13876#issuecomment-1611471103

   @jolshan Yes, the main reason for wrapping was consistency. However, I'm now 
considering a different kind of consistency - never wrap fatal errors - which 
is what was originally suggested in the KIP.


-- 
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



[GitHub] [kafka] lucasbru commented on pull request #13876: KAFKA-10733: Clean up producer exceptions

2023-06-21 Thread via GitHub


lucasbru commented on PR #13876:
URL: https://github.com/apache/kafka/pull/13876#issuecomment-1600924404

   > 1. We might end up breaking customer's application code for exception 
handling with this change. What is our plan to prevent the inadvertent impact 
to customer's code on upgrade?
   > 2. This should probably be accompanied with changes in docs and notable 
change section for 3.6.0 - 
[kafka.apache.org/documentation.html#upgrade_350_notable](https://kafka.apache.org/documentation.html#upgrade_350_notable)
   
@divijvaidya 
   
   - To quote the accepted KIP, "This is a pure client side change which only 
affects the resiliency of new Producer client and Streams. For customized EOS 
use case, user needs to change their exception catching logic to take actions 
against their exception handling ... However, all the thrown exceptions' base 
type would still be KafkaException, so the effect should be minimal."
   
   However, I think you have a point that in the case of 
`ProducerFencedException` / `InvalidProducerEpochException`, we should be 
careful, as it is something that the user may catch frequently. And the KIP 
itself seems to have two conflicting views on this, authored by two different 
developers @abbccdda and @guozhangwang . In the original form, the KIP proposed 
to never wrap fatal errors, while the updated version proposed to wrap 
`ProducerFencedException` and `InvalidProducerEpochException`. 


-- 
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