Re: Review Request 26710: Patch for KAFKA-1637
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/#review56699 --- core/src/main/scala/kafka/server/KafkaApis.scala https://reviews.apache.org/r/26710/#comment97084 I think it's better to use the metadata cache here. - Neha Narkhede On Oct. 14, 2014, 10:04 p.m., Ewen Cheslack-Postava wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/ --- (Updated Oct. 14, 2014, 10:04 p.m.) Review request for kafka. Bugs: KAFKA-1637 https://issues.apache.org/jira/browse/KAFKA-1637 Repository: kafka Description --- KAFKA-1637 Return correct error code and offsets for OffsetFetchRequest for unknown topics/partitions vs no associated consumer. Diffs - core/src/main/scala/kafka/common/OffsetMetadataAndError.scala 1586243d20d6a181a1bd9f07e1c9493596005b32 core/src/main/scala/kafka/server/KafkaApis.scala 67f2833804cb15976680e42b9dc49e275c89d266 core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 2d9325045ac1ac2d7531161b32c98c847125cbf0 Diff: https://reviews.apache.org/r/26710/diff/ Testing --- Thanks, Ewen Cheslack-Postava
Re: Review Request 26710: Patch for KAFKA-1637
On Oct. 15, 2014, 5:22 a.m., Joel Koshy wrote: core/src/main/scala/kafka/server/KafkaApis.scala, line 510 https://reviews.apache.org/r/26710/diff/1/?file=721124#file721124line510 There is an issue here. The replica manager only contains information about partitions that are assigned to this broker. However, some consumer group's offset manager may also be on this broker and that group may consume various partitions that are not assigned to this broker. The offset manager though will still contain offsets for those partitions. Wups. My bad. Didn't want to push this patch, but the other one. Also have a suggestion on the same code you commented on. - Neha --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/#review56662 --- On Oct. 14, 2014, 10:04 p.m., Ewen Cheslack-Postava wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/ --- (Updated Oct. 14, 2014, 10:04 p.m.) Review request for kafka. Bugs: KAFKA-1637 https://issues.apache.org/jira/browse/KAFKA-1637 Repository: kafka Description --- KAFKA-1637 Return correct error code and offsets for OffsetFetchRequest for unknown topics/partitions vs no associated consumer. Diffs - core/src/main/scala/kafka/common/OffsetMetadataAndError.scala 1586243d20d6a181a1bd9f07e1c9493596005b32 core/src/main/scala/kafka/server/KafkaApis.scala 67f2833804cb15976680e42b9dc49e275c89d266 core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 2d9325045ac1ac2d7531161b32c98c847125cbf0 Diff: https://reviews.apache.org/r/26710/diff/ Testing --- Thanks, Ewen Cheslack-Postava
Re: Review Request 26710: Patch for KAFKA-1637
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/ --- (Updated Oct. 15, 2014, 4:08 p.m.) Review request for kafka. Bugs: KAFKA-1637 https://issues.apache.org/jira/browse/KAFKA-1637 Repository: kafka Description (updated) --- Use MetadataCache instead of ReplicaManager to check for non-existent topics and partitions. Diffs (updated) - core/src/main/scala/kafka/common/OffsetMetadataAndError.scala 1586243d20d6a181a1bd9f07e1c9493596005b32 core/src/main/scala/kafka/server/KafkaApis.scala 67f2833804cb15976680e42b9dc49e275c89d266 core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 2d9325045ac1ac2d7531161b32c98c847125cbf0 Diff: https://reviews.apache.org/r/26710/diff/ Testing --- Thanks, Ewen Cheslack-Postava
Re: Review Request 26710: Patch for KAFKA-1637
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/#review56802 --- Ship it! Minor naming comment which we can take care of on check-in. core/src/main/scala/kafka/server/KafkaApis.scala https://reviews.apache.org/r/26710/#comment97223 How about val (unknownTopicPartitions, knownTopicPartitions) core/src/main/scala/kafka/server/KafkaApis.scala https://reviews.apache.org/r/26710/#comment97224 Same here. - Joel Koshy On Oct. 15, 2014, 4:08 p.m., Ewen Cheslack-Postava wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/ --- (Updated Oct. 15, 2014, 4:08 p.m.) Review request for kafka. Bugs: KAFKA-1637 https://issues.apache.org/jira/browse/KAFKA-1637 Repository: kafka Description --- Use MetadataCache instead of ReplicaManager to check for non-existent topics and partitions. Diffs - core/src/main/scala/kafka/common/OffsetMetadataAndError.scala 1586243d20d6a181a1bd9f07e1c9493596005b32 core/src/main/scala/kafka/server/KafkaApis.scala 67f2833804cb15976680e42b9dc49e275c89d266 core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 2d9325045ac1ac2d7531161b32c98c847125cbf0 Diff: https://reviews.apache.org/r/26710/diff/ Testing --- Thanks, Ewen Cheslack-Postava
Re: Review Request 26710: Patch for KAFKA-1637
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/ --- (Updated Oct. 15, 2014, 9:47 p.m.) Review request for kafka. Bugs: KAFKA-1637 https://issues.apache.org/jira/browse/KAFKA-1637 Repository: kafka Description (updated) --- Use MetadataCache instead of ReplicaManager to check for non-existent topics and partitions. Updating naming Diffs (updated) - core/src/main/scala/kafka/common/OffsetMetadataAndError.scala 1586243d20d6a181a1bd9f07e1c9493596005b32 core/src/main/scala/kafka/server/KafkaApis.scala 67f2833804cb15976680e42b9dc49e275c89d266 core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 2d9325045ac1ac2d7531161b32c98c847125cbf0 Diff: https://reviews.apache.org/r/26710/diff/ Testing --- Thanks, Ewen Cheslack-Postava
Review Request 26710: Patch for KAFKA-1637
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/ --- Review request for kafka. Bugs: KAFKA-1637 https://issues.apache.org/jira/browse/KAFKA-1637 Repository: kafka Description --- KAFKA-1637 Return correct error code and offsets for OffsetFetchRequest for unknown topics/partitions vs no associated consumer. Diffs - core/src/main/scala/kafka/common/OffsetMetadataAndError.scala 1586243d20d6a181a1bd9f07e1c9493596005b32 core/src/main/scala/kafka/server/KafkaApis.scala 67f2833804cb15976680e42b9dc49e275c89d266 core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 2d9325045ac1ac2d7531161b32c98c847125cbf0 Diff: https://reviews.apache.org/r/26710/diff/ Testing --- Thanks, Ewen Cheslack-Postava
Re: Review Request 26710: Patch for KAFKA-1637
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/#review56655 --- Ship it! Ship It! - Neha Narkhede On Oct. 14, 2014, 10:04 p.m., Ewen Cheslack-Postava wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/ --- (Updated Oct. 14, 2014, 10:04 p.m.) Review request for kafka. Bugs: KAFKA-1637 https://issues.apache.org/jira/browse/KAFKA-1637 Repository: kafka Description --- KAFKA-1637 Return correct error code and offsets for OffsetFetchRequest for unknown topics/partitions vs no associated consumer. Diffs - core/src/main/scala/kafka/common/OffsetMetadataAndError.scala 1586243d20d6a181a1bd9f07e1c9493596005b32 core/src/main/scala/kafka/server/KafkaApis.scala 67f2833804cb15976680e42b9dc49e275c89d266 core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 2d9325045ac1ac2d7531161b32c98c847125cbf0 Diff: https://reviews.apache.org/r/26710/diff/ Testing --- Thanks, Ewen Cheslack-Postava
Re: Review Request 26710: Patch for KAFKA-1637
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/#review56662 --- core/src/main/scala/kafka/server/KafkaApis.scala https://reviews.apache.org/r/26710/#comment97067 There is an issue here. The replica manager only contains information about partitions that are assigned to this broker. However, some consumer group's offset manager may also be on this broker and that group may consume various partitions that are not assigned to this broker. The offset manager though will still contain offsets for those partitions. - Joel Koshy On Oct. 14, 2014, 10:04 p.m., Ewen Cheslack-Postava wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26710/ --- (Updated Oct. 14, 2014, 10:04 p.m.) Review request for kafka. Bugs: KAFKA-1637 https://issues.apache.org/jira/browse/KAFKA-1637 Repository: kafka Description --- KAFKA-1637 Return correct error code and offsets for OffsetFetchRequest for unknown topics/partitions vs no associated consumer. Diffs - core/src/main/scala/kafka/common/OffsetMetadataAndError.scala 1586243d20d6a181a1bd9f07e1c9493596005b32 core/src/main/scala/kafka/server/KafkaApis.scala 67f2833804cb15976680e42b9dc49e275c89d266 core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala 2d9325045ac1ac2d7531161b32c98c847125cbf0 Diff: https://reviews.apache.org/r/26710/diff/ Testing --- Thanks, Ewen Cheslack-Postava
