Re: Review Request 31591: Patch for KAFKA-1992

2015-03-06 Thread Gwen Shapira 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
---

(Updated March 6, 2015, 9:37 p.m.)


Review request for kafka.


Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992


Repository: kafka


Description (updated)
---

add logging per Jiangjie Qin comment


revert unintentional changes to log4j


merge with trunk


few small fixes suggested by Jun


tiny typo


formatting


Diffs (updated)
-

  core/src/main/scala/kafka/cluster/Partition.scala 
c4bf48a801007ebe7497077d2018d6dffe1677d4 
  core/src/main/scala/kafka/server/DelayedProduce.scala 
4d763bf05efb24a394662721292fc54d32467969 

Diff: https://reviews.apache.org/r/31591/diff/


Testing
---


Thanks,

Gwen Shapira

Re: Review Request 31591: Patch for KAFKA-1992

2015-03-06 Thread Gwen Shapira 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
---

(Updated March 6, 2015, 9:36 p.m.)


Review request for kafka.


Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992


Repository: kafka


Description (updated)
---

add logging per Jiangjie Qin comment


revert unintentional changes to log4j


merge with trunk


few small fixes suggested by Jun


tiny typo


Diffs (updated)
-

  core/src/main/scala/kafka/cluster/Partition.scala 
c4bf48a801007ebe7497077d2018d6dffe1677d4 
  core/src/main/scala/kafka/server/DelayedProduce.scala 
4d763bf05efb24a394662721292fc54d32467969 

Diff: https://reviews.apache.org/r/31591/diff/


Testing
---


Thanks,

Gwen Shapira

Re: Review Request 31591: Patch for KAFKA-1992

2015-03-06 Thread Gwen Shapira 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
---

(Updated March 6, 2015, 9:34 p.m.)


Review request for kafka.


Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992


Repository: kafka


Description (updated)
---

add logging per Jiangjie Qin comment


revert unintentional changes to log4j


merge with trunk


few small fixes suggested by Jun


Diffs (updated)
-

  core/src/main/scala/kafka/cluster/Partition.scala 
c4bf48a801007ebe7497077d2018d6dffe1677d4 
  core/src/main/scala/kafka/server/DelayedProduce.scala 
4d763bf05efb24a394662721292fc54d32467969 

Diff: https://reviews.apache.org/r/31591/diff/


Testing
---


Thanks,

Gwen Shapira

Re: Review Request 31591: Patch for KAFKA-1992

2015-03-03 Thread Jun Rao 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/#review75120
---


Thanks for the patch. A few minor comments below.


core/src/main/scala/kafka/cluster/Partition.scala


The last %s should be %d.



core/src/main/scala/kafka/cluster/Partition.scala


Our current coding convention is not to wrap single line statement in {}.



core/src/main/scala/kafka/cluster/Partition.scala


Space should be added after comma.



core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala


This change is already committed.


- Jun Rao


On March 4, 2015, 1:17 a.m., Gwen Shapira wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31591/
> ---
> 
> (Updated March 4, 2015, 1:17 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1992
> https://issues.apache.org/jira/browse/KAFKA-1992
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> add logging per Jiangjie Qin comment
> 
> 
> revert unintentional changes to log4j
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> c4bf48a801007ebe7497077d2018d6dffe1677d4 
>   core/src/main/scala/kafka/server/DelayedProduce.scala 
> 4d763bf05efb24a394662721292fc54d32467969 
>   core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 
> ba48a636dd0b0ed06646d56bb36aa3d43228604f 
> 
> Diff: https://reviews.apache.org/r/31591/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gwen Shapira
> 
>

Re: Review Request 31591: Patch for KAFKA-1992

2015-03-03 Thread Gwen Shapira 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
---

(Updated March 4, 2015, 1:17 a.m.)


Review request for kafka.


Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992


Repository: kafka


Description (updated)
---

add logging per Jiangjie Qin comment


revert unintentional changes to log4j


Diffs (updated)
-

  core/src/main/scala/kafka/cluster/Partition.scala 
c4bf48a801007ebe7497077d2018d6dffe1677d4 
  core/src/main/scala/kafka/server/DelayedProduce.scala 
4d763bf05efb24a394662721292fc54d32467969 
  core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 
ba48a636dd0b0ed06646d56bb36aa3d43228604f 

Diff: https://reviews.apache.org/r/31591/diff/


Testing
---


Thanks,

Gwen Shapira

Re: Review Request 31591: Patch for KAFKA-1992

2015-03-03 Thread Gwen Shapira 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
---

(Updated March 3, 2015, 10:16 p.m.)


Review request for kafka.


Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992


Repository: kafka


Description (updated)
---

add logging per Jiangjie Qin comment


Diffs (updated)
-

  core/src/main/scala/kafka/cluster/Partition.scala 
c4bf48a801007ebe7497077d2018d6dffe1677d4 
  core/src/main/scala/kafka/server/DelayedProduce.scala 
4d763bf05efb24a394662721292fc54d32467969 
  core/src/test/resources/log4j.properties 
1b7d5d8f7d5fae7d272849715714781cad05d77b 
  core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 
ba48a636dd0b0ed06646d56bb36aa3d43228604f 

Diff: https://reviews.apache.org/r/31591/diff/


Testing
---


Thanks,

Gwen Shapira

Re: Review Request 31591: Patch for KAFKA-1992

2015-03-02 Thread Jiangjie Qin 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/#review74827
---

Ship it!


LGTM, just a minor comment.


core/src/main/scala/kafka/cluster/Partition.scala


This part seems now serving only logging purpose. If that is the case, can 
we make it even clearer. For example, print all the acked replicas instead of 
just a number.


- Jiangjie Qin


On March 1, 2015, 7:58 a.m., Gwen Shapira wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31591/
> ---
> 
> (Updated March 1, 2015, 7:58 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1992
> https://issues.apache.org/jira/browse/KAFKA-1992
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> remove unnecessary requiredAcks parameter and clean up few comments
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> c4bf48a801007ebe7497077d2018d6dffe1677d4 
>   core/src/main/scala/kafka/server/DelayedProduce.scala 
> 4d763bf05efb24a394662721292fc54d32467969 
> 
> Diff: https://reviews.apache.org/r/31591/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gwen Shapira
> 
>

Re: Review Request 31591: Patch for KAFKA-1992

2015-03-01 Thread Jeff Holoman 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/#review74699
---


Looks good to me.

- Jeff Holoman


On March 1, 2015, 7:58 a.m., Gwen Shapira wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31591/
> ---
> 
> (Updated March 1, 2015, 7:58 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1992
> https://issues.apache.org/jira/browse/KAFKA-1992
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> remove unnecessary requiredAcks parameter and clean up few comments
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> c4bf48a801007ebe7497077d2018d6dffe1677d4 
>   core/src/main/scala/kafka/server/DelayedProduce.scala 
> 4d763bf05efb24a394662721292fc54d32467969 
> 
> Diff: https://reviews.apache.org/r/31591/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gwen Shapira
> 
>

Review Request 31591: Patch for KAFKA-1992

2015-02-28 Thread Gwen Shapira 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31591/
---

Review request for kafka.


Bugs: KAFKA-1992
https://issues.apache.org/jira/browse/KAFKA-1992


Repository: kafka


Description
---

remove unnecessary requiredAcks parameter and clean up few comments


Diffs
-

  core/src/main/scala/kafka/cluster/Partition.scala 
c4bf48a801007ebe7497077d2018d6dffe1677d4 
  core/src/main/scala/kafka/server/DelayedProduce.scala 
4d763bf05efb24a394662721292fc54d32467969 

Diff: https://reviews.apache.org/r/31591/diff/


Testing
---


Thanks,

Gwen Shapira