twitter-archive / commons

Twitter common libraries for python and the JVM (deprecated)
http://twitter.github.com/commons
Other
2.1k stars 565 forks source link

Fix Zookeeper group member listener leak in leader election #423

Closed sttts closed 4 years ago

sttts commented 8 years ago

The Candidate implementation leaks GroupChangeListener objects when abdicating leadership. These objects are registered as watches for group members and are never deregistered. Consequently, one gets more and more ugly messages like

"Current member ID %s is not a candidate for leader, current voting: %s"

in the logs.

This change only creates one watch for the Candidate implementation and reused it to avoid the leak (note: there is no way to actually remove a watch that has been registered before).

Fixes https://github.com/mesosphere/marathon/issues/2419.

jsirois commented 8 years ago

@sttts - TravisCI is red on an unrelated pants build error and it looks like its been so for some time. The twitter/commons is not maintained as far as I'm aware; ie: even if you get your change in, it probably won't be published as a jar anytime soon. Hopefully you have a way to consume this change w/o relying on acceptance/publishing. If not, or even if so, you may be interested in the series of changes tracked in https://issues.apache.org/jira/browse/AURORA-1468. I'll be posting the one directly relevant to this PR today, but the series converts Aurora from twitter/commons Candidate (via SingletonService), to Apache Curator's LeaderLatch recipe.

sttts commented 8 years ago

@jsirois thanks for info and the link. Fortunately, we are in a similar situation, moving on to curator. This is more like a shortterm fix. We have forked the file anyway, so having it here unmerged is not an issue.

CLAassistant commented 5 years ago

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

grimreaper commented 4 years ago

Thank you for your contribution. Unfortunately, we’re not going to continue maintaining twitter-commons and are archiving all pull requests.