----------------------------------------------------------- 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 question I have is if the new behavior is what we want if the broker sees a request with requiredAcks > 1. Ideally this shouldn't happen, but someone could be using an earlier version of the client that accepted that setting and could independently update brokers to this newer version that doesn't support those values. (Alternatively, though unrelated to the original issue, a bad client library might allow an invalid value.) It looks like it'll now eventually fail (but slowly) since the value of requiredAcks isn't validated by the broker and checkEnoughReplicasReachOffset will never return true. I think the broker should validate the requiredAcks value when the message is received, failing fast if the value isn't 0, 1, or negative. Then the DelayedProduce is only used if the value is negative and checkEnoughReplicasReachOffset shouldn't even take the requiredAcks parameter. - Ewen Cheslack-Postava On Jan. 7, 2015, 12:20 a.m., Gwen Shapira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29647/ > ----------------------------------------------------------- > > (Updated Jan. 7, 2015, 12:20 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1697 > https://issues.apache.org/jira/browse/KAFKA-1697 > > > Repository: kafka > > > Description > ------- > > removed broker code for handling acks>1 and made > NotEnoughReplicasAfterAppendException non-retriable > > > Diffs > ----- > > > clients/src/main/java/org/apache/kafka/common/errors/NotEnoughReplicasAfterAppendException.java > 75c80a97e43089cb3f924a38f86d67b5a8dd2b89 > core/src/main/scala/kafka/cluster/Partition.scala > b230e9a1fb1a3161f4c9d164e4890a16eceb2ad4 > > Diff: https://reviews.apache.org/r/29647/diff/ > > > Testing > ------- > > > Thanks, > > Gwen Shapira > >
