You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by "Henry Robinson (JIRA)" <ji...@apache.org> on 2009/10/09 01:20:31 UTC

[jira] Created: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
---------------------------------------------------------------------------------------------------

                 Key: ZOOKEEPER-549
                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
             Project: Zookeeper
          Issue Type: Improvement
          Components: quorum, server
    Affects Versions: 3.2.1
            Reporter: Henry Robinson
            Assignee: Henry Robinson
             Fix For: 3.3.0


For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henry Robinson updated ZOOKEEPER-549:
-------------------------------------

    Attachment: ZOOKEEPER-549.patch

This patch renames the Peer* classes to Learner*, fixes the downcast issue and removes some unused imports. I haven't moved the packages around, just to keep this patch manageable. 



> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12764577#action_12764577 ] 

Hadoop QA commented on ZOOKEEPER-549:
-------------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12421844/ZOOKEEPER-549.patch
  against trunk revision 823371.

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 3 new or modified tests.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac compiler warnings.

    -1 findbugs.  The patch appears to introduce 2 new Findbugs warnings.

    +1 release audit.  The applied patch does not increase the total number of release audit warnings.

    +1 core tests.  The patch passed core unit tests.

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/21/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/21/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/21/console

This message is automatically generated.

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henry Robinson updated ZOOKEEPER-549:
-------------------------------------

    Status: Patch Available  (was: Open)

This patch refactors Followers into a Peer->Follower hierarchy in preparation for the upcoming Observers patch (ZOOKEEPER-368). 

There's no new functionality in this patch, save for the introduction of the getView API in QuorumPeer which is tested by two tests in QuorumTest. All tests pass when this patch is applied against trunk.

For ease of review, I've listed the main changes below:

1. A Peer class has been introduced which is a superclass of Follower (and, eventually, Observer). Functionality common to all peers has been moved into this class.
2. Follower.followLeader has been refactored into a driver for three methods contained in Peer. These three methods are: connectToLeader(InetSocketAddress), registerWithLeader(int pktType) and syncWithLeader(long newLeaderzxid). Observers will contain their own driver method which calls these three methods in the superclass.
3. Similarly, FollowerZooKeeperHandler has been refactored into PeerZooKeeperHandler and FollowerZooKeeperHandler.
4. FollowerHandler has been renamed PeerHandler. The sock variable has been made protected and is accessed via getSocket().
5. ZooKeeperServer.getTouchSnapshot has been made protected as it is not called outside its own class.
6. FollowerCnxAcceptor has been renamed PeerCnxAcceptor
7. QuorumPeer has a new getView API which returns the internally held map of QuorumPeers, plus a viewContains method to determine if a server is in that map.



> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henry Robinson updated ZOOKEEPER-549:
-------------------------------------

    Attachment: ZOOKEEPER-549.patch

Updated findbugs.xml to mask these warnings. Findbugs goes back to 0 for me locally.

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12771676#action_12771676 ] 

Mahadev konar commented on ZOOKEEPER-549:
-----------------------------------------

I tried a bunch of permutations of the new code (after this patch) servers and old code  servers just to make sure we are backwards compatible. It all works fine. I will try committing this ASAP after a final review.

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768250#action_12768250 ] 

Flavio Paiva Junqueira commented on ZOOKEEPER-549:
--------------------------------------------------

Nice job, Henry! I have just a quick question about FOLLOWERINFO. I've noticed that in your patch one of the Learner methods is registerWithLeader(), and this method takes a packet type. I'm assuming that you have not replaced FOLLOWERINFO with LEARNERINFO, and that registerWithLeader takes a packet type as parameter because observers will send something like OBSERVERINFO?

Assuming that what I said is correct, what if learners send LEARNERINFO, and the leader verifies the status of the server against its configuration?

I apologize if I'm mixing jiras here. I'm just trying to understand why FOLLOWERINFO hasn'changed.

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768458#action_12768458 ] 

Patrick Hunt commented on ZOOKEEPER-549:
----------------------------------------

To put a fine point on that - ensure that the refactored code is b/w compatible with the original code. That should
be a general goal as well as an item in the patch reviewers checklist.


> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768622#action_12768622 ] 

Flavio Paiva Junqueira commented on ZOOKEEPER-549:
--------------------------------------------------

Just to make my position clear. I initially questioned why the patch doesn't replace FOLLOWERINFO with LEARNERINFO (which would be b/w compatible given that we keep the same value for the constant), and I was trying to understand why Henry didn't do it. After our discussion (above), it became clear that we should leave it for the observer jira.



> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768344#action_12768344 ] 

Henry Robinson commented on ZOOKEEPER-549:
------------------------------------------

You're right - the observers patch will introduce OBSERVERINFO which is the way that the leader learns to distinguish observers from followers. 

It's also possible that the Leader check its configuration and assigns the appropriate role to the Learner. However, I'd still like to have the role negotiated between Learner and Leader and run-time (otherwise we'd have to check for misconfiguration all throughout the code because an observer would silently be treated like a follower, never vote and that would be hard to debug). 

In the observers patch, I'll add some code so that if the Leader receives OBSERVERINFO or FOLLOWERINFO it checks that this is the expected type from that particular peer.



> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768116#action_12768116 ] 

Hadoop QA commented on ZOOKEEPER-549:
-------------------------------------

+1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12422775/ZOOKEEPER-549.patch
  against trunk revision 826787.

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 6 new or modified tests.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac compiler warnings.

    +1 findbugs.  The patch does not introduce any new Findbugs warnings.

    +1 release audit.  The applied patch does not increase the total number of release audit warnings.

    +1 core tests.  The patch passed core unit tests.

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/33/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/33/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/33/console

This message is automatically generated.

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12764847#action_12764847 ] 

Henry Robinson commented on ZOOKEEPER-549:
------------------------------------------

Both findbugs warnings were, I think, present in previous builds - they've just moved. 

One is the use of System.exit(13), and the other is an unused SessionExpirer member.



> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-549:
-----------------------------------

    Status: Open  (was: Patch Available)

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henry Robinson updated ZOOKEEPER-549:
-------------------------------------

    Attachment: ZOOKEEPER-549.patch

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henry Robinson updated ZOOKEEPER-549:
-------------------------------------

    Status: Open  (was: Patch Available)

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henry Robinson updated ZOOKEEPER-549:
-------------------------------------

    Attachment: ZOOKEEPER-549.patch

Argh, renamed the files that the findbugs exclusions were in, but didn't update the findbugs xml file. Sorry, hudson, try again. 

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henry Robinson updated ZOOKEEPER-549:
-------------------------------------

    Status: Patch Available  (was: Open)

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768386#action_12768386 ] 

Flavio Paiva Junqueira commented on ZOOKEEPER-549:
--------------------------------------------------

I originally thought that by using LEANERINFO for both observers and followers, we would make the protocol backward compatible. I now realize that it makes no difference. 

Once we add observers, if we have an observer trying to connect to an old leader, even if we use LEANERINFO (with the same value 11 we have for FOLLOWERINFO in trunk), it won't work because the leader will expect acks from that observer. An old server can also connect as a follower to a new leader without a problem if we keep FOLLOWERINFO as is. We would have a problem if the follower were configured as an observer on the leader, but I suppose this is a configuration error.

My conclusion is that I don't see an issue with using OBSERVERINFO or FOLLOWERINFO regarding backward compatibility. The only issue I see is that having OBSERVERINFO creates yet another message and there is a way around it. I personally don't think it is a big deal to add another message, but some folks might not like it.

On my end, +1 for the patch. We can continue the discussion on adding OBSERVERINFO or not in the observer jira. For this patch, I think it is good as is.

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12766724#action_12766724 ] 

Henry Robinson commented on ZOOKEEPER-549:
------------------------------------------

Mahadev / Flavio -  

Thanks for the comments!

I agree about renaming from Peer; Learner isn't a bad name. If no-one has a better suggestion, I'll do that :)

Good point on the downcasts, I'll remove the unnecessary ones. Also good catch on the imports; I rely on Eclipse to catch them and it sometimes doesn't. 

Definitely agree on the testing; but since this is a rearrangement of code it's not clear how to write many more functional tests since we already have some coverage. Is anyone running a stress test workload? I've just got my hands on some resources to do something similar, but setting it up would take a bit of time. 

I think there are a host of refactorings that could still stand to be done on this code and elsewhere. I agree on the package split, that probably belongs in this JIRA.






> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12766096#action_12766096 ] 

Flavio Paiva Junqueira commented on ZOOKEEPER-549:
--------------------------------------------------

Thanks, Henry, the patch looks good. I have a couple of high level comments:

* I think we should call "Peer*" classes something else because calling them Peer gives the idea that it contains functionality that belongs to leaders, followers, and observers. Leader does not extend Peer, though. Also, we already have a QuorumPeer class (and related classes), which makes it. In my interpretation, the common functionality between followers and observers is that they commit (learn) transactions, so we could call it a Committer or a Learner. I personally prefer the latter, and I'm obviously interested in other suggestions;
* Most likely this should be a separate jira, but I'd like to propose that split the package structure further, in particular for the quorum package. I suggest: quorum.leader, quorum.peer, quorum.learner, quorum.le. 

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12771747#action_12771747 ] 

Mahadev konar commented on ZOOKEEPER-549:
-----------------------------------------

+1 this looks good.... Ill commit this tonight... 

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12764969#action_12764969 ] 

Hadoop QA commented on ZOOKEEPER-549:
-------------------------------------

+1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12421942/ZOOKEEPER-549.patch
  against trunk revision 823371.

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 6 new or modified tests.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac compiler warnings.

    +1 findbugs.  The patch does not introduce any new Findbugs warnings.

    +1 release audit.  The applied patch does not increase the total number of release audit warnings.

    +1 core tests.  The patch passed core unit tests.

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/22/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/22/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/22/console

This message is automatically generated.

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768106#action_12768106 ] 

Hadoop QA commented on ZOOKEEPER-549:
-------------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12422770/ZOOKEEPER-549.patch
  against trunk revision 826787.

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 6 new or modified tests.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac compiler warnings.

    -1 findbugs.  The patch appears to introduce 2 new Findbugs warnings.

    +1 release audit.  The applied patch does not increase the total number of release audit warnings.

    +1 core tests.  The patch passed core unit tests.

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/32/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/32/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/32/console

This message is automatically generated.

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henry Robinson updated ZOOKEEPER-549:
-------------------------------------

    Status: Patch Available  (was: Open)

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12766716#action_12766716 ] 

Mahadev konar commented on ZOOKEEPER-549:
-----------------------------------------

henry,
 good to see this. This is much needed refactoring. Some comments/questions-

 - +1 on flavio's suggestion. I couldnt find a better name then Peer though :). We do have to keep the Followers as Followers because that we use for the ones that have a say in the proposals!

 - there is an unwanted import in Follower.java and more in other classes like FollowerZooKeepeerServer and maybe others.
  {code}
  import org.apache.zookeeper.txn.TxnHeader;
  {code}

- downcast operations in Follower.java for downcasting to FollowerZooKeeperServer. Its done on each and every read of packet and processing. There is some cost to it.

  {quote}
  Downcast operations (also called narrowing conversions in the Java Language Specification) convert an ancestor class reference to a subclass reference. This casting operation creates execution overhead, since Java requires that the cast be checked at runtime to make sure that it's valid.
 {quote}
 from javaworld.
 I think you should be able to downcast once and use that object from then on?

-  other then that, we might want to consider refactoring all the classes into subpackages like flavio suggested

- also, we should make sure we do enough testing because this is a lot of refactoring.


> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Hunt updated ZOOKEEPER-549:
-----------------------------------

    Status: Open  (was: Patch Available)

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12764858#action_12764858 ] 

Patrick Hunt commented on ZOOKEEPER-549:
----------------------------------------

No, the current build has 0 findbugs warnings (and we want to keep it that way) -- if you use the exclusion list that's in svn. 

It may be the case that the exclusion matcher is no longer matching, if you say, moved some code around. You may need to update
the findbugs exclusion list as part of the patch.


> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768466#action_12768466 ] 

Henry Robinson commented on ZOOKEEPER-549:
------------------------------------------

Mahadev - yes, the FOLLOWERINFO discussion really applies to the observer patch. I think that we should push that specific backwards compatibility discussion onto the observers jira. 

I believe backwards compatibility is completely preserved by this patch: there aren't any protocol changes. 



> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768450#action_12768450 ] 

Mahadev konar commented on ZOOKEEPER-549:
-----------------------------------------

the patch looks good but I am getting confused with the OBSERVERINFO/FOLLOWERINFO discussion above... As of this patch is the above discussion relavant? As far as I can tell its just a refactoring and the servers before and after this patch should be able to work with each other ... right?

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henry Robinson updated ZOOKEEPER-549:
-------------------------------------

    Status: Patch Available  (was: Open)

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12772231#action_12772231 ] 

Hudson commented on ZOOKEEPER-549:
----------------------------------

Integrated in ZooKeeper-trunk #514 (See [http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/514/])
    . Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers (henry robinson via mahadev)


> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (ZOOKEEPER-549) Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers

Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/ZOOKEEPER-549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mahadev konar updated ZOOKEEPER-549:
------------------------------------

      Resolution: Fixed
    Release Note: Refactor followers code into Learner from which followers and observers will inherit.
    Hadoop Flags: [Reviewed]
          Status: Resolved  (was: Patch Available)

I just committed this. thanks henry.

> Refactor Followers and related classes into a Peer->Follower hierarchy in preparation for Observers
> ---------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-549
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-549
>             Project: Zookeeper
>          Issue Type: Improvement
>          Components: quorum, server
>    Affects Versions: 3.2.1
>            Reporter: Henry Robinson
>            Assignee: Henry Robinson
>             Fix For: 3.3.0
>
>         Attachments: ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch, ZOOKEEPER-549.patch
>
>
> For the Observers patch (ZOOKEEPER-368), a lot of functionality is shared between Followers and Observers. To avoid copying code, it makes sense to push the common code into a parent Peer class and specialise it for Followers and Observers. At the same time, some of the lengthier methods in Follower can be broken up to make the code more readable. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.