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-2479 #98

Closed
wants to merge 3 commits into from
Closed

ZOOKEEPER-2479 #98

wants to merge 3 commits into from

Conversation

rakeshadr
Copy link
Contributor

No description provided.

* Keeps time taken for leader election in milliseconds. Sets the value to
* this variable only after the completion of leader election.
*/
private long electionTimeTaken = -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

sincere question: does it make sense to make this field volatile? I know there are some concurrency guarantees but not sure if it's worth change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @eribeiro ,

LeaderMXBean, FollowerMXBean will be available only after the quorum leader election and the value won't be changed until next LE. I think, adding 'volatile' doesn't make any difference, right?

code reference:

https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/quorum/Leader.java#L417
https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/quorum/Follower.java#L70

Leader.java
    zk.registerJMX(new LeaderBean(this, zk), self.jmxLocalPeerBean);

Follower.java
    fzk.registerJMX(new FollowerBean(this, zk), self.jmxLocalPeerBean);

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, agree. :) Thanks for explaining!

@fpj
Copy link
Contributor

fpj commented Nov 15, 2016

+1, LGTM. @eribeiro is this ready according to you?

@rakeshadr
Copy link
Contributor Author

@eribeiro ,, appreciate your feedback. Thanks!

String bean = "";
if (qp.getPeerState() == ServerState.FOLLOWING) {
bean = CommonNames.DOMAIN + ":name0=ReplicatedServer_id" + i
+ ",name1=replica." + i + ",name2=Follower";
Copy link
Member

Choose a reason for hiding this comment

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

hyper nit: String.format() reads better than +

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @rgs1, I've have modified the test code as per your suggestion. Please take another look at the new commits.

electionTimeTaken >= 0);
} else if (qp.getPeerState() == ServerState.LEADING) {
bean = CommonNames.DOMAIN + ":name0=ReplicatedServer_id" + i
+ ",name1=replica." + i + ",name2=Leader";
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

@eribeiro eribeiro left a comment

Choose a reason for hiding this comment

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

+1 LGTM Rakesh and Flavio.

@asfgit asfgit closed this in 8616a9e Dec 20, 2016
asfgit pushed a commit that referenced this pull request Dec 20, 2016
…lowerMXBean

Author: Rakesh Radhakrishnan <rakeshr@apache.org>

Reviewers: Edward Ribeiro <edward.ribeiro@gmail.com>, fpj <fpj@apache.org>

Closes #98 from rakeshadr/ZK-2479 and squashes the following commits:

2c9eadf [Rakesh Radhakrishnan] ZOOKEEPER-2479: avoids duplicate code
5af1314 [Rakesh Radhakrishnan] ZOOKEEPER-2479: Fixed Raul's review comments
cc011a0 [Rakesh Radhakrishnan] ZOOKEEPER-2479: Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean

(cherry picked from commit 8616a9e)
Signed-off-by: fpj <fpj@apache.org>
lvfangmin pushed a commit to lvfangmin/zookeeper that referenced this pull request Jun 17, 2018
…lowerMXBean

Author: Rakesh Radhakrishnan <rakeshr@apache.org>

Reviewers: Edward Ribeiro <edward.ribeiro@gmail.com>, fpj <fpj@apache.org>

Closes apache#98 from rakeshadr/ZK-2479 and squashes the following commits:

2c9eadf [Rakesh Radhakrishnan] ZOOKEEPER-2479: avoids duplicate code
5af1314 [Rakesh Radhakrishnan] ZOOKEEPER-2479: Fixed Raul's review comments
cc011a0 [Rakesh Radhakrishnan] ZOOKEEPER-2479: Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
…lowerMXBean

Author: Rakesh Radhakrishnan <rakeshr@apache.org>

Reviewers: Edward Ribeiro <edward.ribeiro@gmail.com>, fpj <fpj@apache.org>

Closes apache#98 from rakeshadr/ZK-2479 and squashes the following commits:

2c9eadf [Rakesh Radhakrishnan] ZOOKEEPER-2479: avoids duplicate code
5af1314 [Rakesh Radhakrishnan] ZOOKEEPER-2479: Fixed Raul's review comments
cc011a0 [Rakesh Radhakrishnan] ZOOKEEPER-2479: Add 'electionTimeTaken' value in LeaderMXBean and FollowerMXBean
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants