You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/11/10 11:08:43 UTC

[GitHub] [kafka] soceanainn opened a new pull request #11482: Align behaviour for producer callbacks with documented behaviour

soceanainn opened a new pull request #11482:
URL: https://github.com/apache/kafka/pull/11482


   Originally, Callback would return a null metadata value when an error occurred.
   
   This was partially changed by [KAFKA-3303](https://issues.apache.org/jira/browse/KAFKA-3303), where in some cases Callback would return an 'empty' metadata. In this empty metadata TopicPartition is set correctly but all other fields are set as `-1`.
   
   The docs were later updated by [KAFKA-7412](https://issues.apache.org/jira/browse/KAFKA-7412), but it incorrectly states that Callback will always return this 'empty' metadata when an error occurs. However in the case of any exceptions that are a subclass of ApiException, Callback will still return a null value (see [here](https://github.com/apache/kafka/blob/3.1/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java#L1002)).
   
   This change fixes the behaviour when an ApiException is thrown, to align that behaviour with other exceptions and with the currently documented behaviour.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] soceanainn commented on pull request #11482: MINOR: Align behaviour for producer callbacks with documented behaviour

Posted by GitBox <gi...@apache.org>.
soceanainn commented on pull request #11482:
URL: https://github.com/apache/kafka/pull/11482#issuecomment-965658710


   @junrao can you grant permission for me to create a KIP for this change?


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] soceanainn commented on pull request #11482: KAFKA-13448: Align behaviour for producer callbacks with documented behaviour

Posted by GitBox <gi...@apache.org>.
soceanainn commented on pull request #11482:
URL: https://github.com/apache/kafka/pull/11482#issuecomment-1032743124


   An equivalent change was implemented and merged in #11689 


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] soceanainn commented on pull request #11482: MINOR: Align behaviour for producer callbacks with documented behaviour

Posted by GitBox <gi...@apache.org>.
soceanainn commented on pull request #11482:
URL: https://github.com/apache/kafka/pull/11482#issuecomment-965666855


   @junrao it is seamus.oceanainn


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] junrao commented on pull request #11482: MINOR: Align behaviour for producer callbacks with documented behaviour

Posted by GitBox <gi...@apache.org>.
junrao commented on pull request #11482:
URL: https://github.com/apache/kafka/pull/11482#issuecomment-965661445


   @soceanainn : What's your wiki ID? Thanks.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] soceanainn closed pull request #11482: KAFKA-13448: Align behaviour for producer callbacks with documented behaviour

Posted by GitBox <gi...@apache.org>.
soceanainn closed pull request #11482:
URL: https://github.com/apache/kafka/pull/11482


   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] junrao commented on pull request #11482: KAFKA-13448: Align behaviour for producer callbacks with documented behaviour

Posted by GitBox <gi...@apache.org>.
junrao commented on pull request #11482:
URL: https://github.com/apache/kafka/pull/11482#issuecomment-965877866


   @soceanainn : Just gave you the wiki permission.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] soceanainn commented on pull request #11482: KAFKA-13448: Align behaviour for producer callbacks with documented behaviour

Posted by GitBox <gi...@apache.org>.
soceanainn commented on pull request #11482:
URL: https://github.com/apache/kafka/pull/11482#issuecomment-965889575


   @junrao thanks, I'll try to create a KIP tomorrow.
   
   I've added a test for the ApiException case, however for the other case (using `InterceptorCallback`), I can't find a way to test it from the KafkaProducer class.
   
   It's easier to test from the 'ProducerBatch' side of things, but this leads to complications since it is testing that metadata is correctly set to null (e.g. [here](
   https://github.com/apache/kafka/blob/trunk/clients/src/test/java/org/apache/kafka/clients/producer/internals/ProducerBatchTest.java#L125)).
   
   The easiest fix is to make InterceptorCallback accessible at the package level, and update the ProducerBatch method to accept InterceptorCallback as input instead of Callback. That way it would make this implicit conversion from null metadata to an 'empty' record more obvious, and allow us to write proper unit tests around this. What do you think?


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] junrao commented on pull request #11482: KAFKA-13448: Align behaviour for producer callbacks with documented behaviour

Posted by GitBox <gi...@apache.org>.
junrao commented on pull request #11482:
URL: https://github.com/apache/kafka/pull/11482#issuecomment-969213761


   @soceanainn : Thanks for the KIP.  Making ProducerBatch accept InterceptorCallback seems fine since it's an internal class.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org