KAFKA-7412: onComplete should not reassign metadata variable#5798
KAFKA-7412: onComplete should not reassign metadata variable#5798junrao merged 3 commits intoapache:trunkfrom
metadata variable#5798Conversation
|
@hachikuji Please review this minor change. Thanks. |
|
Well, if this is intentional, we should refine the doc instead. |
|
Yeah, I agree. |
|
@junrao Could you clarify if this is intentional？ |
|
@huxihx : This does seem to be intentional. The following is the commit message for KAFKA-3303. So, we just want to update the javadoc accordingly, i.e, when exception is not null in the callback, metadata will contain the special -1 value for all fields except for topicParition, which will be valid. "Currently, If sending the record fails before it gets to the server, ProducerInterceptor.onAcknowledgement() is called with metadata == null, and non-null exception. However, it is useful to pass topic and partition, if known, to ProducerInterceptor.onAcknowledgement() as well. This patch ensures that ProducerInterceptor.onAcknowledgement() gets record metadata with topic and maybe partition. If partition is not set in 'record' and KafkaProducer.send() fails before partition gets assigned, then ProducerInterceptor.onAcknowledgement() gets RecordMetadata with partition == -1. Only time when ProducerInterceptor.onAcknowledgement() gets null record metadata is when the client passes null record to KafkaProducer.send()." |
…ata`. The Java doc for `InterceptorCallback#onComplete` says that exactly one of the arguments(metadata and exception) will be non-null. However, the commitment will be broken when TimeoutException is encountered since the code reassigns a new-created RecordMetadata object to variable `metadata`. The solution is to leave `metadata1` unchanged and pass a new RecordMetadata instance to `onAcknowledgement`.
|
@junrao Please review the updated doc. Thanks. |
| * metadata will contain the special -1 value for all fields except for topicPartition, which will be valid. | ||
| * | ||
| * @param metadata The metadata for the record that was sent (i.e. the partition and offset). An empty metadata | ||
| * with -1 value for all fields except for topicPartition returned if an error occurred. |
There was a problem hiding this comment.
returned => will be returned
The metadata in the callback is not null with non-null exception. Reviewers: Jun Rao <junrao@gmail.com>
3 participants
The Java doc for
InterceptorCallback#onCompletesays that exactly one of the arguments(metadata and exception) will be non-null. However, the commitment will be broken when TimeoutException is encountered since the code reassigns a new-created RecordMetadata object to variablemetadata.The solution is to leave
metadata1unchanged and pass a new RecordMetadata instance toonAcknowledgement.More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.
Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.
Committer Checklist (excluded from commit message)