Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ZOOKEEPER-3203] Tracking the number of non voting followers in ZK #722

Closed
wants to merge 1 commit into from

Conversation

lvfangmin
Copy link
Contributor

No description provided.

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall okay

I left a comment

public String nonVotingFollowerInfo() {
StringBuilder sb = new StringBuilder();
for (LearnerHandler handler : leader.getNonVotingFollowers()) {
sb.append(handler.toString()).append("\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to have a specific toStringForJMX method.

In the future people will change the toString method without thinking that they are changing the output of a public endpoint

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point, but I'd like to keep the same behavior in this patch as what we're doing now for the followInfo in this LeaderBean.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I think we can live with it

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lvfangmin
Copy link
Contributor Author

retest this please

Copy link
Contributor

@anmolnar anmolnar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@asfgit asfgit closed this in 9e30955 Jan 7, 2019
@anmolnar
Copy link
Contributor

anmolnar commented Jan 7, 2019

Merged to master branch. Thanks @lvfangmin !

RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
Author: Fangmin Lyu <fangmin@apache.org>

Reviewers: andor@apache.org

Closes apache#722 from lvfangmin/ZOOKEEPER-3203
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants