Re: Review Request 26770: Patch for KAFKA-1108

2014-10-16 Thread Neha Narkhede

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



core/src/main/scala/kafka/server/KafkaServer.scala
https://reviews.apache.org/r/26770/#comment97399

Should this be WARN instead? ERROR wouldn't be ideal since this operation 
is retried later. Also wondering if this message actually gives much 
information about the reason of the failure? It might just print out 
IOException. I think the reason for failure that people might understand is 
what might cause the IOException. How about improving the error message by 
saying that the possible cause for this error could be that the leader movement 
operation on the controller took longer than than the configured 
socket.timeout.ms. 

This will encourage users to inspect if the socket.timeout.ms needs to be 
bumped up or inspect why the controller is taking long for moving the leaders 
away from this broker.


- Neha Narkhede


On Oct. 15, 2014, 6:55 p.m., Ewen Cheslack-Postava wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26770/
 ---
 
 (Updated Oct. 15, 2014, 6:55 p.m.)
 
 
 Review request for kafka.
 
 
 Bugs: KAFKA-1108
 https://issues.apache.org/jira/browse/KAFKA-1108
 
 
 Repository: kafka
 
 
 Description
 ---
 
 KAFKA-1108 Log IOException messages during controlled shutdown.
 
 
 Diffs
 -
 
   core/src/main/scala/kafka/server/KafkaServer.scala 
 07c0a078ffa5142441f687da851472da732c3837 
 
 Diff: https://reviews.apache.org/r/26770/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ewen Cheslack-Postava
 




Re: Review Request 26770: Patch for KAFKA-1108

2014-10-16 Thread Ewen Cheslack-Postava


 On Oct. 16, 2014, 5:55 p.m., Neha Narkhede wrote:
  core/src/main/scala/kafka/server/KafkaServer.scala, line 239
  https://reviews.apache.org/r/26770/diff/1/?file=722474#file722474line239
 
  Should this be WARN instead? ERROR wouldn't be ideal since this 
  operation is retried later. Also wondering if this message actually gives 
  much information about the reason of the failure? It might just print out 
  IOException. I think the reason for failure that people might understand is 
  what might cause the IOException. How about improving the error message by 
  saying that the possible cause for this error could be that the leader 
  movement operation on the controller took longer than than the configured 
  socket.timeout.ms. 
  
  This will encourage users to inspect if the socket.timeout.ms needs to 
  be bumped up or inspect why the controller is taking long for moving the 
  leaders away from this broker.

The INFO level just matched similar messages a few lines above, although this 
is a more significant issue than those. Newest patch updates to WARN. Message 
is also more detailed, but ideally the IOException message also contains more 
than just the class name.


- Ewen


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


On Oct. 16, 2014, 8:53 p.m., Ewen Cheslack-Postava wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26770/
 ---
 
 (Updated Oct. 16, 2014, 8:53 p.m.)
 
 
 Review request for kafka.
 
 
 Bugs: KAFKA-1108
 https://issues.apache.org/jira/browse/KAFKA-1108
 
 
 Repository: kafka
 
 
 Description
 ---
 
 More informative message and increase log level to warn.
 
 
 Diffs
 -
 
   core/src/main/scala/kafka/server/KafkaServer.scala 
 07c0a078ffa5142441f687da851472da732c3837 
 
 Diff: https://reviews.apache.org/r/26770/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ewen Cheslack-Postava
 




Review Request 26770: Patch for KAFKA-1108

2014-10-15 Thread Ewen Cheslack-Postava

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

Review request for kafka.


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


Repository: kafka


Description
---

KAFKA-1108 Log IOException messages during controlled shutdown.


Diffs
-

  core/src/main/scala/kafka/server/KafkaServer.scala 
07c0a078ffa5142441f687da851472da732c3837 

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


Testing
---


Thanks,

Ewen Cheslack-Postava