Review Request 24992: Patch for KAFKA-1609
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/ --- Review request for kafka. Bugs: KAFKA-1609 https://issues.apache.org/jira/browse/KAFKA-1609 Repository: kafka Description --- KAFKA-1609; Metadata response should only exclude a PartitionInfo when its error is LEADER_NOT_AVAILABLE Diffs - clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 7d90fceffd4a92460c616f8bacd78057a38a3c16 Diff: https://reviews.apache.org/r/24992/diff/ Testing --- Thanks, Dong Lin
Re: Review Request 24992: Patch for KAFKA-1609
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/ --- (Updated Aug. 22, 2014, 8:02 p.m.) Review request for kafka. Bugs: KAFKA-1609 https://issues.apache.org/jira/browse/KAFKA-1609 Repository: kafka Description (updated) --- KAFKA-1609; New producer metadata response handling should only exclude a PartitionInfo when its error is LEADER_NOT_AVAILABLE Diffs - clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 7d90fceffd4a92460c616f8bacd78057a38a3c16 Diff: https://reviews.apache.org/r/24992/diff/ Testing --- Thanks, Dong Lin
Re: Review Request 24992: Patch for KAFKA-1609
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/ --- (Updated Aug. 22, 2014, 8:04 p.m.) Review request for kafka. Bugs: KAFKA-1609 https://issues.apache.org/jira/browse/KAFKA-1609 Repository: kafka Description (updated) --- KAFKA-1609; New producer metadata response handling should only exclude a PartitionInfo when its error is LEADER_NOT_AVAILABLE Diffs (updated) - clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 7d90fceffd4a92460c616f8bacd78057a38a3c16 Diff: https://reviews.apache.org/r/24992/diff/ Testing --- Thanks, Dong Lin
Re: Review Request 24992: Patch for KAFKA-1609
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/#review51316 --- clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java https://reviews.apache.org/r/24992/#comment89505 Is this possilbe that the reponse can also contain other exceptions like UNKNOWN_TOPIC_OR_PARTITION now or in the future? If yes we also need to exlude them also. - Guozhang Wang On Aug. 22, 2014, 8:04 p.m., Dong Lin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/ --- (Updated Aug. 22, 2014, 8:04 p.m.) Review request for kafka. Bugs: KAFKA-1609 https://issues.apache.org/jira/browse/KAFKA-1609 Repository: kafka Description --- KAFKA-1609; New producer metadata response handling should only exclude a PartitionInfo when its error is LEADER_NOT_AVAILABLE Diffs - clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 7d90fceffd4a92460c616f8bacd78057a38a3c16 Diff: https://reviews.apache.org/r/24992/diff/ Testing --- Thanks, Dong Lin
Re: Review Request 24992: Patch for KAFKA-1609
On Aug. 22, 2014, 8:56 p.m., Joel Koshy wrote: clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java, line 121 https://reviews.apache.org/r/24992/diff/2/?file=667650#file667650line121 Guozhang, an unknown topic would be captured at the topic-error level (above) no? Unknown partition should not happen since we don't support partition deletion and anyway currently don't query metadata at the partition-level. I agree. - Dong --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/#review51317 --- On Aug. 22, 2014, 8:04 p.m., Dong Lin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/ --- (Updated Aug. 22, 2014, 8:04 p.m.) Review request for kafka. Bugs: KAFKA-1609 https://issues.apache.org/jira/browse/KAFKA-1609 Repository: kafka Description --- KAFKA-1609; New producer metadata response handling should only exclude a PartitionInfo when its error is LEADER_NOT_AVAILABLE Diffs - clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 7d90fceffd4a92460c616f8bacd78057a38a3c16 Diff: https://reviews.apache.org/r/24992/diff/ Testing --- Thanks, Dong Lin
Re: Review Request 24992: Patch for KAFKA-1609
On Aug. 22, 2014, 8:50 p.m., Guozhang Wang wrote: clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java, line 121 https://reviews.apache.org/r/24992/diff/2/?file=667650#file667650line121 Is this possilbe that the reponse can also contain other exceptions like UNKNOWN_TOPIC_OR_PARTITION now or in the future? If yes we also need to exlude them also. I think the UNKNOWN_TOPIC_OR_PARTITION will be captured in topic error code. From what I learned from source code, currently metadata response only returns LEADER_NOT_AVAILABLE or REPLICA_NOT_AVAILABLE as parition error code. - Dong --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/#review51316 --- On Aug. 22, 2014, 8:04 p.m., Dong Lin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/ --- (Updated Aug. 22, 2014, 8:04 p.m.) Review request for kafka. Bugs: KAFKA-1609 https://issues.apache.org/jira/browse/KAFKA-1609 Repository: kafka Description --- KAFKA-1609; New producer metadata response handling should only exclude a PartitionInfo when its error is LEADER_NOT_AVAILABLE Diffs - clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 7d90fceffd4a92460c616f8bacd78057a38a3c16 Diff: https://reviews.apache.org/r/24992/diff/ Testing --- Thanks, Dong Lin
Re: Review Request 24992: Patch for KAFKA-1609
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/#review51325 --- Ship it! Ship It! - Guozhang Wang On Aug. 22, 2014, 8:04 p.m., Dong Lin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/ --- (Updated Aug. 22, 2014, 8:04 p.m.) Review request for kafka. Bugs: KAFKA-1609 https://issues.apache.org/jira/browse/KAFKA-1609 Repository: kafka Description --- KAFKA-1609; New producer metadata response handling should only exclude a PartitionInfo when its error is LEADER_NOT_AVAILABLE Diffs - clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 7d90fceffd4a92460c616f8bacd78057a38a3c16 Diff: https://reviews.apache.org/r/24992/diff/ Testing --- Thanks, Dong Lin
Re: Review Request 24992: Patch for KAFKA-1609
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/#review51327 --- clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java https://reviews.apache.org/r/24992/#comment89517 Would it be simpler to just don't do the test on partError? The rest of the code can deal with a null leader already. It's probably a bit weird to ignore a partition whose leader is not available. For example, if a message has to go to a partition (e.g., keyed message), we want to know that partition whether the leader is available or not. - Jun Rao On Aug. 22, 2014, 8:04 p.m., Dong Lin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/ --- (Updated Aug. 22, 2014, 8:04 p.m.) Review request for kafka. Bugs: KAFKA-1609 https://issues.apache.org/jira/browse/KAFKA-1609 Repository: kafka Description --- KAFKA-1609; New producer metadata response handling should only exclude a PartitionInfo when its error is LEADER_NOT_AVAILABLE Diffs - clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 7d90fceffd4a92460c616f8bacd78057a38a3c16 Diff: https://reviews.apache.org/r/24992/diff/ Testing --- Thanks, Dong Lin
Re: Review Request 24992: Patch for KAFKA-1609
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/ --- (Updated Aug. 22, 2014, 11:22 p.m.) Review request for kafka. Bugs: KAFKA-1609 https://issues.apache.org/jira/browse/KAFKA-1609 Repository: kafka Description (updated) --- KAFKA-1609; New producer metadata response handling should not exclude a PartitionInfo when there is partition error. Diffs (updated) - clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 7d90fceffd4a92460c616f8bacd78057a38a3c16 Diff: https://reviews.apache.org/r/24992/diff/ Testing --- Thanks, Dong Lin
Re: Review Request 24992: Patch for KAFKA-1609
On Aug. 22, 2014, 10:39 p.m., Jun Rao wrote: clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java, line 121 https://reviews.apache.org/r/24992/diff/2/?file=667650#file667650line121 Would it be simpler to just don't do the test on partError? The rest of the code can deal with a null leader already. It's probably a bit weird to ignore a partition whose leader is not available. For example, if a message has to go to a partition (e.g., keyed message), we want to know that partition whether the leader is available or not. Sure! The patch is updated. - Dong --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/#review51327 --- On Aug. 22, 2014, 11:22 p.m., Dong Lin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24992/ --- (Updated Aug. 22, 2014, 11:22 p.m.) Review request for kafka. Bugs: KAFKA-1609 https://issues.apache.org/jira/browse/KAFKA-1609 Repository: kafka Description --- KAFKA-1609; New producer metadata response handling should not exclude a PartitionInfo when there is partition error. Diffs - clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java 7d90fceffd4a92460c616f8bacd78057a38a3c16 Diff: https://reviews.apache.org/r/24992/diff/ Testing --- Thanks, Dong Lin
