KAFKA-4510: StreamThread must finish rebalance in state PENDING_SHUTDOWN #2227
Conversation
| @@ -389,6 +395,10 @@ private void shutdown() { | |||
| log.error("{} Failed to close restore consumer: ", logPrefix, e); | |||
| } | |||
|
|
|||
| // hotfix to improve ZK behavior als long as KAFKA-4060 is not fixed (c.f. KAFKA-4369) | |||
| // when removing this, make StreamPartitionAssignor#internalTopicManager "private" again | |||
| partitionAssignor.internalTopicManager.zkClient.close(); | |||
There was a problem hiding this comment.
What does this buy us? Not clear from comment.
There was a problem hiding this comment.
It's not really related to this PR. But as reported in https://issues.apache.org/jira/browse/KAFKA-4369 right now we get a lot of this messages in the log:
[2016-07-14 09:49:21,790] WARN [main-SendThread(127.0.0.1:61385)] Session 0x155e860e62b0007 for server null, unexpected error, closing socket connection and attempting reconnect (org.apache.zookeeper.ClientCnxn)
java.net.ConnectException: Connection refused
at sun.nio.ch.SocketChannelImpl.checkConnect(Native Method)
at sun.nio.ch.SocketChannelImpl.finishConnect(SocketChannelImpl.java:717)
at org.apache.zookeeper.ClientCnxnSocketNIO.doTransport(ClientCnxnSocketNIO.java:361)
at org.apache.zookeeper.ClientCnxn$SendThread.run(ClientCnxn.java:1022)
As we want to remove ZK dependency, nobody picked up this bug and it's super annoying if you run test locally in a loop. We get rid of those log messages with this fix. It's not a completely nice fix (but only temporary so it should be ok) but it does the trick -- and I am not sure how long it will take to remove ZK dependency. Thus I though it might be worth to include it for now. Will get removed when ZK dependency gets removed.
There was a problem hiding this comment.
Generally speaking I'd suggest not having a single PR involved with multiple fixes, since it will make reviewing much harder. But since KAFKA-4369 will not be fixed this may be OK. Could you add a TODO at the beginning to make it more visible?
There was a problem hiding this comment.
Generally speaking I'd suggest not having a single PR involved with multiple fixes, since it will make reviewing much harder. But since KAFKA-4369 will not be fixed this may be OK. Could you add a TODO at the beginning to make it more visible?
|
LGTM overall, just one minor comment. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
@guozhangwang Updated. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Merged to trunk. |
Author: Matthias J. Sax <matthias@confluent.io> Reviewers: Eno Thereska, Guozhang Wang Closes apache#2227 from mjsax/kafka-4510-finish-rebalance-on-shutdown
No description provided.