[jira] [Commented] (RATIS-94) Expose basic information over JMX

2017-08-11 Thread Tsz Wo Nicholas Sze (JIRA) 

[ 
https://issues.apache.org/jira/browse/RATIS-94?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16123870#comment-16123870
 ] 

Tsz Wo Nicholas Sze commented on RATIS-94:
--

Filed RATIS-103.

> Expose basic information over JMX
> -
>
> Key: RATIS-94
> URL: https://issues.apache.org/jira/browse/RATIS-94
> Project: Ratis
>  Issue Type: Improvement
>Reporter: Elek, Marton
>Assignee: Elek, Marton
> Fix For: 0.2.0-alpha
>
> Attachments: RATIS-94-1.patch, RATIS-94.2.patch, RATIS-94.3.patch, 
> RATIS-94.4.patch
>
>
> To make it easier to debug the current state of the nodes the basic 
> RatisServer information should be exposed over the JMX interface. Such as: 
> role (leader,follower), latest term, index, follower peers (in case of LEADER)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (RATIS-94) Expose basic information over JMX

2017-08-11 Thread Tsz Wo Nicholas Sze (JIRA) 

[ 
https://issues.apache.org/jira/browse/RATIS-94?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16123847#comment-16123847
 ] 

Tsz Wo Nicholas Sze commented on RATIS-94:
--

[~elek], thanks a lot for the contribution!

The CopyOnWriteArrayList may not work well in LeaderState since it does not 
support remove.   :)

Now, RaftReconfigurationBaseTest.testRemovePeers() fails with 
UnsupportedOperationException.
{code}
Exception in thread "Thread-102" java.lang.UnsupportedOperationException
at 
java.util.concurrent.CopyOnWriteArrayList$COWIterator.remove(CopyOnWriteArrayList.java:1176)
at 
org.apache.ratis.server.impl.LeaderState.updateSenders(LeaderState.java:254)
at 
org.apache.ratis.server.impl.LeaderState.replicateNewConf(LeaderState.java:483)
at 
org.apache.ratis.server.impl.LeaderState.checkAndUpdateConfiguration(LeaderState.java:448)
at 
org.apache.ratis.server.impl.LeaderState.updateLastCommitted(LeaderState.java:431)
at 
org.apache.ratis.server.impl.LeaderState.handleEvent(LeaderState.java:330)
at 
org.apache.ratis.server.impl.LeaderState.access$500(LeaderState.java:49)
at 
org.apache.ratis.server.impl.LeaderState$EventProcessor.run(LeaderState.java:298)
{code}


> Expose basic information over JMX
> -
>
> Key: RATIS-94
> URL: https://issues.apache.org/jira/browse/RATIS-94
> Project: Ratis
>  Issue Type: Improvement
>Reporter: Elek, Marton
>Assignee: Elek, Marton
> Fix For: 0.2.0-alpha
>
> Attachments: RATIS-94-1.patch, RATIS-94.2.patch, RATIS-94.3.patch, 
> RATIS-94.4.patch
>
>
> To make it easier to debug the current state of the nodes the basic 
> RatisServer information should be exposed over the JMX interface. Such as: 
> role (leader,follower), latest term, index, follower peers (in case of LEADER)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (RATIS-94) Expose basic information over JMX

2017-08-10 Thread Jing Zhao (JIRA) 

[ 
https://issues.apache.org/jira/browse/RATIS-94?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16122869#comment-16122869
 ] 

Jing Zhao commented on RATIS-94:


+1 will commit the patch shortly.

> Expose basic information over JMX
> -
>
> Key: RATIS-94
> URL: https://issues.apache.org/jira/browse/RATIS-94
> Project: Ratis
>  Issue Type: Improvement
>Reporter: Elek, Marton
>Assignee: Elek, Marton
> Attachments: RATIS-94-1.patch, RATIS-94.2.patch, RATIS-94.3.patch, 
> RATIS-94.4.patch
>
>
> To make it easier to debug the current state of the nodes the basic 
> RatisServer information should be exposed over the JMX interface. Such as: 
> role (leader,follower), latest term, index, follower peers (in case of LEADER)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (RATIS-94) Expose basic information over JMX

2017-08-08 Thread Elek, Marton (JIRA) 

[ 
https://issues.apache.org/jira/browse/RATIS-94?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16118525#comment-16118525
 ] 

Elek, Marton commented on RATIS-94:
---

Rebased and fixed.

> Expose basic information over JMX
> -
>
> Key: RATIS-94
> URL: https://issues.apache.org/jira/browse/RATIS-94
> Project: Ratis
>  Issue Type: Improvement
>Reporter: Elek, Marton
>Assignee: Elek, Marton
> Attachments: RATIS-94-1.patch, RATIS-94.2.patch, RATIS-94.3.patch, 
> RATIS-94.4.patch
>
>
> To make it easier to debug the current state of the nodes the basic 
> RatisServer information should be exposed over the JMX interface. Such as: 
> role (leader,follower), latest term, index, follower peers (in case of LEADER)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (RATIS-94) Expose basic information over JMX

2017-08-07 Thread Jing Zhao (JIRA) 

[ 
https://issues.apache.org/jira/browse/RATIS-94?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16116931#comment-16116931
 ] 

Jing Zhao commented on RATIS-94:


Besides, you may need to rebase the patch... Thanks.

> Expose basic information over JMX
> -
>
> Key: RATIS-94
> URL: https://issues.apache.org/jira/browse/RATIS-94
> Project: Ratis
>  Issue Type: Improvement
>Reporter: Elek, Marton
>Assignee: Elek, Marton
> Attachments: RATIS-94-1.patch, RATIS-94.2.patch, RATIS-94.3.patch
>
>
> To make it easier to debug the current state of the nodes the basic 
> RatisServer information should be exposed over the JMX interface. Such as: 
> role (leader,follower), latest term, index, follower peers (in case of LEADER)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (RATIS-94) Expose basic information over JMX

2017-08-07 Thread Jing Zhao (JIRA) 

[ 
https://issues.apache.org/jira/browse/RATIS-94?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16116927#comment-16116927
 ] 

Jing Zhao commented on RATIS-94:


Yes RaftPeer may be better. BTW,
{code}
972   .map(leaderState1 ->
973   leaderState.getFollowers().stream()
{code}
Here on line 973 leaderState should be leaderState1? I suggest to rename 
leaderState1 to leader.

Other than this the patch looks good to me. +1 after addressing the comments.

> Expose basic information over JMX
> -
>
> Key: RATIS-94
> URL: https://issues.apache.org/jira/browse/RATIS-94
> Project: Ratis
>  Issue Type: Improvement
>Reporter: Elek, Marton
>Assignee: Elek, Marton
> Attachments: RATIS-94-1.patch, RATIS-94.2.patch, RATIS-94.3.patch
>
>
> To make it easier to debug the current state of the nodes the basic 
> RatisServer information should be exposed over the JMX interface. Such as: 
> role (leader,follower), latest term, index, follower peers (in case of LEADER)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (RATIS-94) Expose basic information over JMX

2017-08-07 Thread Elek, Marton (JIRA) 

[ 
https://issues.apache.org/jira/browse/RATIS-94?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16116882#comment-16116882
 ] 

Elek, Marton commented on RATIS-94:
---

Thanks the feedback. 

I can accept {{List}} as the return type of {{LeaderState#getSenders}} 
(which is IMHO not getSenders any more but rather just getFollowers), but what 
do you think about {{List}}? {{RaftPeer}} is immutable so it couldn't 
be modified from outside and little bit more meaningfull/more structured then 
the simple String. 

> Expose basic information over JMX
> -
>
> Key: RATIS-94
> URL: https://issues.apache.org/jira/browse/RATIS-94
> Project: Ratis
>  Issue Type: Improvement
>Reporter: Elek, Marton
>Assignee: Elek, Marton
> Attachments: RATIS-94-1.patch, RATIS-94.2.patch, RATIS-94.3.patch
>
>
> To make it easier to debug the current state of the nodes the basic 
> RatisServer information should be exposed over the JMX interface. Such as: 
> role (leader,follower), latest term, index, follower peers (in case of LEADER)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (RATIS-94) Expose basic information over JMX

2017-07-31 Thread Jing Zhao (JIRA) 

[ 
https://issues.apache.org/jira/browse/RATIS-94?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16107935#comment-16107935
 ] 

Jing Zhao commented on RATIS-94:


Thanks for updating the patch, [~elek]. The latest patch looks good to me. Only 
one further comment related to the {{getFollowers}}: copying the leaderState 
reference and getting its follower information should be fine, but we also need 
to be careful about the concurrent ops on the follower list in LeaderState. 
Besides, adding an API which directly returns the original follower list from 
the LeaderState may not be a good idea (from the encapsulation point of view). 
So my suggestion is:
# Change {{LeaderState#senders}} into a synchronized list.
# We can replace {{LeaderState#getSenders}} with a method which returns 
follower information as a List.


> Expose basic information over JMX
> -
>
> Key: RATIS-94
> URL: https://issues.apache.org/jira/browse/RATIS-94
> Project: Ratis
>  Issue Type: Improvement
>Reporter: Elek, Marton
>Assignee: Elek, Marton
> Attachments: RATIS-94-1.patch, RATIS-94.2.patch
>
>
> To make it easier to debug the current state of the nodes the basic 
> RatisServer information should be exposed over the JMX interface. Such as: 
> role (leader,follower), latest term, index, follower peers (in case of LEADER)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (RATIS-94) Expose basic information over JMX

2017-07-28 Thread Elek, Marton (JIRA) 

[ 
https://issues.apache.org/jira/browse/RATIS-94?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16104929#comment-16104929
 ] 

Elek, Marton commented on RATIS-94:
---

Thx, the feedback. I addressed your points and also improved the unit test, to 
be sure I can retrieve attributes.

To things:

1. PeerId couldn't be used directly as we use MXBean (which doesn't require 
interface/class on the caller side), so we need additional MBean interfaces for 
PeerInfo and PeerId, also PeerId contains the id in multiple form. I proposal 
is to use List which could be the representation of the PeerInfo 
(peerInfo.toString: id + address)

2. You are right with the race condition, but I wouldn't add synchronize to the 
getFollowers (getPeers in the original patch) method as any custom monitoring 
tool could check the jmx value frequently which could cause frequent blocking. 
I suggest to copy the leaderState reference (with 
Optional.ofNullable(leaderState)) and return the followers from the reference. 
In case of a race condition we will return the followers even it in that 
millisecond the leaderState is set to null. I think this eventuallity is 
acceptable as it's just the monitoring.  

> Expose basic information over JMX
> -
>
> Key: RATIS-94
> URL: https://issues.apache.org/jira/browse/RATIS-94
> Project: Ratis
>  Issue Type: Improvement
>Reporter: Elek, Marton
>Assignee: Elek, Marton
> Attachments: RATIS-94-1.patch, RATIS-94.2.patch
>
>
> To make it easier to debug the current state of the nodes the basic 
> RatisServer information should be exposed over the JMX interface. Such as: 
> role (leader,follower), latest term, index, follower peers (in case of LEADER)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (RATIS-94) Expose basic information over JMX

2017-07-27 Thread Jing Zhao (JIRA) 

[ 
https://issues.apache.org/jira/browse/RATIS-94?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16104277#comment-16104277
 ] 

Jing Zhao commented on RATIS-94:


Thanks for working on this, [~elek]. The patch looks good to me in general. 
Some comments:
# Currently we're using two spaces for indentation.
# In RaftServerMXBean, please make sure we're using the same javadoc style 
(starting with "/**").
# Can we directly use RaftPeer instead of creating a new RaftPeerInfo?
# "toString().toString()" is redundant. 
{code}
171   ObjectName name =
172   new ObjectName("Ratis:service=RaftServer,group=" + 
getGroupId().toString().toString() + ",id=" + getId().toString());
{code}
# The following code should be "return 
RaftServerImpl.this.getGroupId().toString();".
{code}
959 @Override
960 public String getGroupId() {
961   return getGroupId().toString();
962 }
{code}
# {{getPeers}} can be renamed to {{getFollowers}}?
{code}
/**
 * Addresses of the followers, only for leaders
 */
List getPeers();
{code}
# There may be a race condition in the following code: the leaderState may be 
changed after the null check. 
{code}
@Override
public List getPeers() {
  if (leaderState != null) {
return leaderState.getSenders().stream()
.map(appender -> appender.getFollower())
.map(follower -> new 
RaftPeerInfo(follower.getPeer().getId().toString(), 
follower.getPeer().getAddress().toString()))
.collect(Collectors.toList());
{code}

> Expose basic information over JMX
> -
>
> Key: RATIS-94
> URL: https://issues.apache.org/jira/browse/RATIS-94
> Project: Ratis
>  Issue Type: Improvement
>Reporter: Elek, Marton
>Assignee: Elek, Marton
> Attachments: RATIS-94-1.patch
>
>
> To make it easier to debug the current state of the nodes the basic 
> RatisServer information should be exposed over the JMX interface. Such as: 
> role (leader,follower), latest term, index, follower peers (in case of LEADER)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)