You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Sriharsha Chintalapani <ha...@hortonworks.com> on 2014/08/13 18:11:07 UTC
Review Request 24653: Patch for KAFKA-1577
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24653/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1577
https://issues.apache.org/jira/browse/KAFKA-1577
Repository: kafka
Description
-------
KAFKA-1577. Exception in ConnectionQuotas while shutting down.
Diffs
-----
core/src/main/scala/kafka/network/SocketServer.scala 9693bc0b2d92377ee378e68f25b522bcaa836ddf
Diff: https://reviews.apache.org/r/24653/diff/
Testing
-------
Thanks,
Sriharsha Chintalapani
Re: Review Request 24653: Patch for KAFKA-1577
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24653/#review51194
-----------------------------------------------------------
core/src/main/scala/kafka/network/SocketServer.scala
<https://reviews.apache.org/r/24653/#comment89227>
Actually, I would recommend we don't do this here. During normal operation we should not be seeing this issue. It is only a corner case during shutdown.
Instead I would just recommend wrapping the call to closeAll with a Utils.swallowError - since it (should) only happen during shutdown. If it happens any other time we need to know about it.
Even the shutdown case can be handled with some synchronization but it is probably not worth the extra code.
- Joel Koshy
On Aug. 21, 2014, 2:58 a.m., Sriharsha Chintalapani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24653/
> -----------------------------------------------------------
>
> (Updated Aug. 21, 2014, 2:58 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1577
> https://issues.apache.org/jira/browse/KAFKA-1577
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1577. Exception in ConnectionQuotas while shutting down.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/network/SocketServer.scala 9693bc0b2d92377ee378e68f25b522bcaa836ddf
>
> Diff: https://reviews.apache.org/r/24653/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sriharsha Chintalapani
>
>
Re: Review Request 24653: Patch for KAFKA-1577
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24653/#review51891
-----------------------------------------------------------
core/src/main/scala/kafka/network/SocketServer.scala
<https://reviews.apache.org/r/24653/#comment90566>
Thanks for the patch - this is what I had in mind. The stack trace still shows up in the log though - I forgot that although swallow keeps the exception from bubbling up further, it still logs the exception which is annoying.
However, the shutdown no longer hangs (as it used to) so your patch works.
So I think it would be reasonable to just change this to swallow (which means swallowWarn) and let the exception show up in the logs for now.
There is a separate effort to improve our logging convention and the current recommendation (for this instance) is to avoid showing the stack trace. That will be done in the jiras that come out of https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Error+Handling+and+Logging
- Joel Koshy
On Aug. 26, 2014, 2:33 p.m., Sriharsha Chintalapani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24653/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2014, 2:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1577
> https://issues.apache.org/jira/browse/KAFKA-1577
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1577. Exception in ConnectionQuotas while shutting down.
>
>
> KAFKA-1577. Exception in ConnectionQuotas while shutting down.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/network/SocketServer.scala 9693bc0b2d92377ee378e68f25b522bcaa836ddf
>
> Diff: https://reviews.apache.org/r/24653/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sriharsha Chintalapani
>
>
Re: Review Request 24653: Patch for KAFKA-1577
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24653/#review51905
-----------------------------------------------------------
Ship it!
Ship It!
- Joel Koshy
On Aug. 26, 2014, 2:33 p.m., Sriharsha Chintalapani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24653/
> -----------------------------------------------------------
>
> (Updated Aug. 26, 2014, 2:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1577
> https://issues.apache.org/jira/browse/KAFKA-1577
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1577. Exception in ConnectionQuotas while shutting down.
>
>
> KAFKA-1577. Exception in ConnectionQuotas while shutting down.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/network/SocketServer.scala 9693bc0b2d92377ee378e68f25b522bcaa836ddf
>
> Diff: https://reviews.apache.org/r/24653/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sriharsha Chintalapani
>
>
Re: Review Request 24653: Patch for KAFKA-1577
Posted by Sriharsha Chintalapani <ha...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24653/
-----------------------------------------------------------
(Updated Aug. 26, 2014, 2:33 p.m.)
Review request for kafka.
Bugs: KAFKA-1577
https://issues.apache.org/jira/browse/KAFKA-1577
Repository: kafka
Description (updated)
-------
KAFKA-1577. Exception in ConnectionQuotas while shutting down.
KAFKA-1577. Exception in ConnectionQuotas while shutting down.
Diffs (updated)
-----
core/src/main/scala/kafka/network/SocketServer.scala 9693bc0b2d92377ee378e68f25b522bcaa836ddf
Diff: https://reviews.apache.org/r/24653/diff/
Testing
-------
Thanks,
Sriharsha Chintalapani
Re: Review Request 24653: Patch for KAFKA-1577
Posted by Sriharsha Chintalapani <ha...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24653/
-----------------------------------------------------------
(Updated Aug. 21, 2014, 2:58 a.m.)
Review request for kafka.
Bugs: KAFKA-1577
https://issues.apache.org/jira/browse/KAFKA-1577
Repository: kafka
Description
-------
KAFKA-1577. Exception in ConnectionQuotas while shutting down.
Diffs (updated)
-----
core/src/main/scala/kafka/network/SocketServer.scala 9693bc0b2d92377ee378e68f25b522bcaa836ddf
Diff: https://reviews.apache.org/r/24653/diff/
Testing
-------
Thanks,
Sriharsha Chintalapani
Re: Review Request 24653: Patch for KAFKA-1577
Posted by Sriharsha Chintalapani <ha...@hortonworks.com>.
> On Aug. 13, 2014, 7:44 p.m., Neha Narkhede wrote:
> > core/src/main/scala/kafka/network/SocketServer.scala, line 470
> > <https://reviews.apache.org/r/24653/diff/1/?file=659368#file659368line470>
> >
> > While this may fix the NoSuchElementException, I wonder why the error happened in the first place as it may point to a bug in the way we are using the connection quotas. For example, are we trying to shut down the same socket channel twice somehow?
> >
> > Finding this will also help write a unit test for this, if possible.
Whitespaces issue fixed. As per comments by Joel Koshy this case is hard to reproduce I tried to write a test where sending bunch requests to socketserver and calling shutdown didn't reproduce this issue.
- Sriharsha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24653/#review50495
-----------------------------------------------------------
On Aug. 21, 2014, 2:58 a.m., Sriharsha Chintalapani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24653/
> -----------------------------------------------------------
>
> (Updated Aug. 21, 2014, 2:58 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1577
> https://issues.apache.org/jira/browse/KAFKA-1577
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1577. Exception in ConnectionQuotas while shutting down.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/network/SocketServer.scala 9693bc0b2d92377ee378e68f25b522bcaa836ddf
>
> Diff: https://reviews.apache.org/r/24653/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sriharsha Chintalapani
>
>
Re: Review Request 24653: Patch for KAFKA-1577
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24653/#review50495
-----------------------------------------------------------
I saw a bunch of whitespaces diffs. Do you know why those were introduced in the patch?
core/src/main/scala/kafka/network/SocketServer.scala
<https://reviews.apache.org/r/24653/#comment88321>
While this may fix the NoSuchElementException, I wonder why the error happened in the first place as it may point to a bug in the way we are using the connection quotas. For example, are we trying to shut down the same socket channel twice somehow?
Finding this will also help write a unit test for this, if possible.
- Neha Narkhede
On Aug. 13, 2014, 4:11 p.m., Sriharsha Chintalapani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24653/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2014, 4:11 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1577
> https://issues.apache.org/jira/browse/KAFKA-1577
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1577. Exception in ConnectionQuotas while shutting down.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/network/SocketServer.scala 9693bc0b2d92377ee378e68f25b522bcaa836ddf
>
> Diff: https://reviews.apache.org/r/24653/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sriharsha Chintalapani
>
>