Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 27, 2015, 11:18 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/cluster/Partition.scala, line 289 > > > > > > good point Yep, good catch. Created KAFKA-1992 to fix this. - Gwen

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review74623 --- core/src/main/scala/kafka/cluster/Partition.scala

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review74592 --- Sorry for the late review. Have one comment below. core/src/main/s

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 13, 2015, 3:05 a.m., Joel Koshy wrote: > > KAFKA-1948 changes crept in to the rb, but I can clean that up. Thanks, man :) I think I need lessons on how to do merges and use the patch-review tool... - Gwen --- This is an autom

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review72317 --- Ship it! KAFKA-1948 changes crept in to the rb, but I can clean tha

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/ --- (Updated Feb. 13, 2015, 2:57 a.m.) Review request for kafka. Bugs: KAFKA-1697

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 12, 2015, 2:31 p.m., Joel Koshy wrote: > > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala, > > line 62 > > > > > > Is this change necessary? > > Gwen Shapira wrote: > Nope, but

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 12, 2015, 2:31 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/ReplicaManager.scala, line 271 > > > > > > This is good, but maybe call this canRespondNow ? Since before > > replication won't app

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 12, 2015, 2:31 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/ReplicaManager.scala, line 271 > > > > > > This is good, but maybe call this canRespondNow ? Since before > > replication won't app

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review72156 --- Ship it! Thanks Gwen - a few minor comments that I can fix up - can

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 12, 2015, 4:20 a.m., Joe Stein wrote: > > core/src/main/scala/kafka/server/ReplicaManager.scala, line 307 > > > > > > Could we change this to a match case? > > requiredAcks match { > > case 0 => {} >

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/ --- (Updated Feb. 12, 2015, 7:14 a.m.) Review request for kafka. Bugs: KAFKA-1697

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review72107 --- core/src/main/scala/kafka/server/ReplicaManager.scala

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/ --- (Updated Feb. 12, 2015, 2:47 a.m.) Review request for kafka. Bugs: KAFKA-1697

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/ --- (Updated Feb. 12, 2015, 2:45 a.m.) Review request for kafka. Bugs: KAFKA-1697

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 11, 2015, 1:48 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/errors/NotEnoughReplicasAfterAppendException.java, > > line 23 > > > > > > Pending discussion. > > Gwen Shapira wrote:

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 11, 2015, 1:48 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/errors/NotEnoughReplicasAfterAppendException.java, > > line 23 > > > > > > Pending discussion. > > Gwen Shapira wrote:

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 11, 2015, 1:48 p.m., Joel Koshy wrote: > > core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala, line 96 > > > > > > Do we need this here? IMO, its a good idea to have this in any test that starts n

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review71960 --- clients/src/main/java/org/apache/kafka/common/errors/NotEnoughRepli

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/ --- (Updated Feb. 11, 2015, 1:06 a.m.) Review request for kafka. Bugs: KAFKA-1697

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 10, 2015, 11:34 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/errors/NotEnoughReplicasAfterAppendException.java, > > line 24 > > > > > > Actually this is exactly what I meant. i.e.

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review71874 --- clients/src/main/java/org/apache/kafka/common/errors/NotEnoughRepli

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 10, 2015, 10:58 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/errors/NotEnoughReplicasAfterAppendException.java, > > line 24 > > > > > > I think our interpretation of retriable is

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review71869 --- clients/src/main/java/org/apache/kafka/common/errors/NotEnoughRepli

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 10, 2015, 8:12 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, line 166 > > > > > > Rather than do this here in kafka-apis, how about moving this to > > ReplicaManager.appendMes

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 10, 2015, 8:12 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/server/KafkaApis.scala, line 170 > > > > > > Should we just do Errors.INVALID_REQUIRED_ACKS? lol. yeah, I've no clue what I was trying to

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 10, 2015, 10:10 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/errors/NotEnoughReplicasAfterAppendException.java, > > line 24 > > > > > > Understood, but if someone uses required.ac

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review71857 --- clients/src/main/java/org/apache/kafka/common/errors/NotEnoughRepli

Re: Review Request 29647: Patch for KAFKA-1697

> On Feb. 10, 2015, 8:12 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/errors/NotEnoughReplicasAfterAppendException.java, > > line 24 > > > > > > Can you clarify why this should no longer be

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review71842 --- clients/src/main/java/org/apache/kafka/common/errors/NotEnoughRepli

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review68687 --- core/src/main/scala/kafka/cluster/Partition.scala

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/ --- (Updated Jan. 14, 2015, 11:41 p.m.) Review request for kafka. Bugs: KAFKA-169

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review67489 --- This seems like a small change, but it is not since it changes the b

Re: Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/#review67044 --- It does look like this is all that needs to be removed. Only questio

Review Request 29647: Patch for KAFKA-1697

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29647/ --- Review request for kafka. Bugs: KAFKA-1697 https://issues.apache.org/jira/b