> On Jan. 12, 2015, 7:47 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, line 88 > > <https://reviews.apache.org/r/29692/diff/3/?file=815275#file815275line88> > > > > We should probably also change OffsetCommitRequest.responseFor . The > > issue is that if you get an UnknownTopicOrPartition error right now we > > convert that to a ConsumerCoordinatorNotAvailableCode which does not apply > > for v0. > > > > (BTW, if this patch were for trunk you would not need to do this since > > latest trunk sets the code correctly in the OffsetManager class) > > > > Alternatively, we could just remove the check here but that would be a > > change in behavior.
This doesn't seem to be necessary since in v0, we handle UnknownTopicOrPartition explicitly in handleOffsetCommitRequest() by converting the exception to the right error code. > On Jan. 12, 2015, 7:47 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, line 101 > > <https://reviews.apache.org/r/29692/diff/3/?file=815275#file815275line101> > > > > Should we explicitly version the scala side of OffsetCommitResponse as > > well especially given that the Java version has a v0/v1? E.g., if a client > > proactively checks for the response version... This seems to always send > > back version = 0 in the response Added a comment to indicate that this constructor is for both v0 and v1. - Jun ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29692/#review67667 ----------------------------------------------------------- On Jan. 12, 2015, 10:30 p.m., Jun Rao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29692/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2015, 10:30 p.m.) > > > Review request for kafka. > > > Bugs: kafka-1841 > https://issues.apache.org/jira/browse/kafka-1841 > > > Repository: kafka > > > Description > ------- > > addressing Joel's comments > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java > 7517b879866fc5dad5f8d8ad30636da8bbe7784a > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java > 3ee5cbad55ce836fd04bb954dcf6ef2f9bc3288f > core/src/main/scala/kafka/api/OffsetCommitRequest.scala > 861a6cf11dc6b6431fcbbe9de00c74a122f204bd > core/src/main/scala/kafka/api/OffsetCommitResponse.scala > 624a1c1cc540688ae2b1fb96665696a6084158e5 > core/src/main/scala/kafka/api/OffsetFetchRequest.scala > c7604b9cdeb8f46507f04ed7d35fcb3d5f150713 > core/src/main/scala/kafka/server/KafkaApis.scala > 9a61fcba3b5eeb295676b3ef720c719ef5244642 > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala > cd16ced5465d098be7a60498326b2a98c248f343 > > Diff: https://reviews.apache.org/r/29692/diff/ > > > Testing > ------- > > > Thanks, > > Jun Rao > >
