Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

KAFKA-8910: Incorrect javadoc at KafkaProducer.InterceptorCallback#onCompletion #7337

Merged
merged 1 commit into from Feb 20, 2020
Merged

KAFKA-8910: Incorrect javadoc at KafkaProducer.InterceptorCallback#onCompletion #7337

merged 1 commit into from Feb 20, 2020

Conversation

dongjinleekr
Copy link
Contributor

Fix the inconsistency between KafkaProducer.InterceptorCallback#onCompletion and Callback#onCompletion.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@dongjinleekr
Copy link
Contributor Author

Rebased onto the latest trunk.

@dongjinleekr
Copy link
Contributor Author

Rebased onto the latest trunk.

cc/ @omkreddy @stanislavkozlovski

@dongjinleekr
Copy link
Contributor Author

ping @omkreddy @guozhangwang

@guozhangwang
Copy link
Contributor

@dongjinleekr I also checked the code, and I think the original reporter is right: when an exception is thrown, the metadata would be null (e.g. see completeFutureAndFireCallbacks in ProducerBatch; only in the interceptor-callback which is different from the producer-callback we would create a dummy metadata and trigger the callback.

I think we should fix InterceptorCallback and let the user-callback to be triggered with null metadata still, and then change the javadoc of Callback.

@dongjinleekr
Copy link
Contributor Author

@guozhangwang Oh yes, I also re-checked the code and you are right. I misunderstood the code - the original reporter was right. 😅

@dongjinleekr
Copy link
Contributor Author

@hachikuji Could you merge this PR? 🙃

@guozhangwang
Copy link
Contributor

At first I was wondering why PR only changes the javadoc of DemoCallBack but now I got it was overlapped with a merged #5798 :) Will merge now. Thanks @dongjinleekr

@guozhangwang guozhangwang merged commit b2aec94 into apache:trunk Feb 20, 2020
ijuma added a commit to confluentinc/kafka that referenced this pull request Feb 24, 2020
@ijuma
Merge remote-tracking branch 'apache-github/trunk' into ccs-master
38dec03 
* apache-github/trunk: (23 commits)
  KAFKA-9530; Fix flaky test `testDescribeGroupWithShortInitializationTimeout` (apache#8154)
  HOTFIX: fix NPE in Kafka Streams IQ (apache#8158)
  MINOR: set scala version automatically based on gradle.properties
  KAFKA-9577; SaslClientAuthenticator incorrectly negotiates SASL_HANDSHAKE version (apache#8142)
  KAFKA-9441: Add internal TransactionManager (apache#8105)
  MINOR: Document endpoints for connector topic tracking (KIP-558)
  MINOR: Standby task commit needed when offsets updated (apache#8146)
  KAFKA-9206; Throw KafkaException on CORRUPT_MESSAGE error in Fetch response (apache#8111)
  MINOR: Remove unwanted regexReplace on tests/kafkatest/__init__.py
  KAFKA-9586: Fix errored json filename in ops documentation
  KAFKA-9575: Mention ZooKeeper 3.5.7 upgrade
  KAFKA-9481: Graceful handling TaskMigrated and TaskCorrupted (apache#8058)
  HOTFIX: don't try to remove uninitialized changelogs from assignment & don't prematurely mark task closed (apache#8140)
  MINOR: Fix javadoc at org.apache.kafka.clients.producer.KafkaProducer.InterceptorCallback#onCompletion (apache#7337)
  MINOR: Improve EOS example exception handling (apache#8052)
  MINOR: Fix a number of warnings in clients test (apache#8073)
  MINOR: Update shell scripts to support z/OS system (apache#7913)
  MINOR: Wording fix in Streams DSL docs (apache#5692)
  MINOR: Add missing @test annotation to MetadataTest#testMetadataMerge (apache#8141)
  KAFKA-9533: ValueTransform forwards `null` values (apache#8108)
  ...
ijuma added a commit to confluentinc/kafka that referenced this pull request Feb 24, 2020
@ijuma
Merge remote-tracking branch 'confluent/master' into remove-support-m…
9f774d7 
…etrics-common

* confluent/master: (76 commits)
  KAFKA-9530; Fix flaky test `testDescribeGroupWithShortInitializationTimeout` (apache#8154)
  HOTFIX: fix NPE in Kafka Streams IQ (apache#8158)
  MINOR: set scala version automatically based on gradle.properties
  KAFKA-9577; SaslClientAuthenticator incorrectly negotiates SASL_HANDSHAKE version (apache#8142)
  KAFKA-9441: Add internal TransactionManager (apache#8105)
  MINOR: Document endpoints for connector topic tracking (KIP-558)
  MINOR: Standby task commit needed when offsets updated (apache#8146)
  Changes to migrate to Artifactory (#263)
  KAFKA-9206; Throw KafkaException on CORRUPT_MESSAGE error in Fetch response (apache#8111)
  MINOR: Remove unwanted regexReplace on tests/kafkatest/__init__.py
  KAFKA-9586: Fix errored json filename in ops documentation
  KAFKA-9575: Mention ZooKeeper 3.5.7 upgrade
  KAFKA-9481: Graceful handling TaskMigrated and TaskCorrupted (apache#8058)
  HOTFIX: don't try to remove uninitialized changelogs from assignment & don't prematurely mark task closed (apache#8140)
  MINOR: Fix javadoc at org.apache.kafka.clients.producer.KafkaProducer.InterceptorCallback#onCompletion (apache#7337)
  MINOR: Improve EOS example exception handling (apache#8052)
  MINOR: Fix a number of warnings in clients test (apache#8073)
  MINOR: Update shell scripts to support z/OS system (apache#7913)
  MINOR: Wording fix in Streams DSL docs (apache#5692)
  MINOR: Add missing @test annotation to MetadataTest#testMetadataMerge (apache#8141)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guozhangwang guozhangwang guozhangwang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@dongjinleekr @guozhangwang