[
https://issues.apache.org/jira/browse/KAFKA-3602?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Guozhang Wang updated KAFKA-3602:
---------------------------------
Description:
In the investigation of KAFKA-3358, we found a case where the side effect of
creating record batches in the internal RecordAccumulator.dequeFor()
method caused unintended behavior. The bug in this case an implicit assumption
elsewhere in the code that all record batches correspond to partitions that
have actually been targeted for sends by the user. This assumption was
invalidated when a topic metadata request is sent with no topics, which results
in the metadata from all topics being returned. The end result when that
happened is that the client gets stuck trying to fetch metadata for topics
which were not even used.
Although this particular problem will be fixed by changing the TopicMetadata
request in KIP-4, it probably could have been avoided by making the side-effect
of batch creation clear in the method name. For example, instead of dequeFor(),
we should use something like getOrCreateDeque(). It's more verbose, but it
makes the behavior clear. From a scan of the code, it looks like there are a
couple places where we do not expect the side-effect of batch creation, so we
should fix that too.
was:
In the investigation of KAFKA-3358, we found a case where the side effect of
creating record batches in the internal RecordAccumulator.dequeueFor()
method caused unintended behavior. The bug in this case an implicit assumption
elsewhere in the code that all record batches correspond to partitions that
have actually been targeted for sends by the user. This assumption was
invalidated when a topic metadata request is sent with no topics, which results
in the metadata from all topics being returned. The end result when that
happened is that the client gets stuck trying to fetch metadata for topics
which were not even used.
Although this particular problem will be fixed by changing the TopicMetadata
request in KIP-4, it probably could have been avoided by making the side-effect
of batch creation clear in the method name. For example, instead of dequeFor(),
we should use something like getOrCreateDeque(). It's more verbose, but it
makes the behavior clear. From a scan of the code, it looks like there are a
couple places where we do not expect the side-effect of batch creation, so we
should fix that too.
> Rename RecordAccumulator dequeFor() and ensure proper usage
> -----------------------------------------------------------
>
> Key: KAFKA-3602
> URL: https://issues.apache.org/jira/browse/KAFKA-3602
> Project: Kafka
> Issue Type: Bug
> Reporter: Jason Gustafson
> Assignee: Jason Gustafson
>
> In the investigation of KAFKA-3358, we found a case where the side effect of
> creating record batches in the internal RecordAccumulator.dequeFor()
> method caused unintended behavior. The bug in this case an implicit
> assumption elsewhere in the code that all record batches correspond to
> partitions that have actually been targeted for sends by the user. This
> assumption was invalidated when a topic metadata request is sent with no
> topics, which results in the metadata from all topics being returned. The end
> result when that happened is that the client gets stuck trying to fetch
> metadata for topics which were not even used.
> Although this particular problem will be fixed by changing the TopicMetadata
> request in KIP-4, it probably could have been avoided by making the
> side-effect of batch creation clear in the method name. For example, instead
> of dequeFor(), we should use something like getOrCreateDeque(). It's more
> verbose, but it makes the behavior clear. From a scan of the code, it looks
> like there are a couple places where we do not expect the side-effect of
> batch creation, so we should fix that too.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
