> On Dec. 18, 2013, 1:19 a.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/controller/ControllerChannelManager.scala, line > > 116 > > <https://reviews.apache.org/r/16335/diff/1/?file=399012#file399012line116> > > > > If connectToBroker timed out here, the thread will die here. So shall > > we move it into doWork, and let the first attempt trying to connect?
Actually the entire connectToBroker() is protected by a try call all throwables. It will not kill the thread, but will simply log an error and disconnect from broker. > On Dec. 18, 2013, 1:19 a.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/controller/ControllerChannelManager.scala, line > > 137 > > <https://reviews.apache.org/r/16335/diff/1/?file=399012#file399012line137> > > > > Could we also log the whole request content? Since this happens rarely > > this should not pollute the logs. Included the correlation id instead. The controller logs the entire request before sending it with the correlation id, so we can link the two to find the request. The concern is that if the broker is down, we may log this error several times and logging requests on each attempt will pollute the log > On Dec. 18, 2013, 1:19 a.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/controller/ControllerChannelManager.scala, line > > 139 > > <https://reviews.apache.org/r/16335/diff/1/?file=399012#file399012line139> > > > > In cases when the toBroker is not responsive for a long time, the > > connectToBroker can still throw an exception and hence not being able to > > send the request. Will this be an issue? We will disconnect and retry until the controller receives a 'broker change' listener notifying the broker went down for good, which is when the thread will be shut down. Until then, we will reconnect-retrysend. - Neha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16335/#review30592 ----------------------------------------------------------- On Dec. 18, 2013, 12:29 a.m., Neha Narkhede wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16335/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2013, 12:29 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1187 > https://issues.apache.org/jira/browse/KAFKA-1187 > > > Repository: kafka > > > Description > ------- > > Changes include 1) Handled error connecting while adding broker in controller > 2) Retry sending state change requests in controller's background thread > > > Diffs > ----- > > core/src/main/scala/kafka/controller/ControllerChannelManager.scala > 7991e42aac4e1d9aaa4c8b185b55fd6172a43f83 > core/src/main/scala/kafka/producer/SyncProducer.scala > 419156eb143fb04a305c91c964307a89ba5a82fa > > Diff: https://reviews.apache.org/r/16335/diff/ > > > Testing > ------- > > > Thanks, > > Neha Narkhede > >
