Skip to content
This repository was archived by the owner on Dec 13, 2019. It is now read-only.

Fix Zookeeper group member listener leak in leader election#423

Closed
sttts wants to merge 1 commit intotwitter-archive:masterfrom
sttts:sttts-one-group-watch
Closed

Fix Zookeeper group member listener leak in leader election#423
sttts wants to merge 1 commit intotwitter-archive:masterfrom
sttts:sttts-one-group-watch

Conversation

@sttts
Copy link

@sttts sttts commented Apr 12, 2016

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 d2iq-archive/marathon#2419.

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 d2iq-archive/marathon#2419.
@sttts sttts force-pushed the sttts-one-group-watch branch 7 times, most recently from f7016d5 to 3b99f0b Compare April 12, 2016 14:50
@jsirois
Copy link
Contributor

jsirois commented Apr 12, 2016

@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
Copy link
Author

sttts commented Apr 12, 2016

@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
Copy link

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
Copy link

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

@grimreaper grimreaper closed this Dec 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ZK CandidateImpl instances leaked after leader elections

4 participants