Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.4.0
    • Component/s: server
    • Labels:
      None

      Description

      As per http://wiki.apache.org/hadoop/ZooKeeper/GSoCReadOnlyMode , create ReadOnlyZooKeeperServer which comes into play when peer is partitioned.

      1. ZOOKEEPER-784.patch
        67 kB
        Ketan G
      2. ZOOKEEPER-784.patch
        67 kB
        Sergey Doroshenko
      3. ZOOKEEPER-784.patch
        66 kB
        Sergey Doroshenko
      4. ZOOKEEPER-784.patch
        63 kB
        Sergey Doroshenko
      5. ZOOKEEPER-784.patch
        62 kB
        Sergey Doroshenko
      6. ZOOKEEPER-784.patch
        59 kB
        Sergey Doroshenko
      7. ZOOKEEPER-784.patch
        52 kB
        Sergey Doroshenko
      8. ZOOKEEPER-784.patch
        61 kB
        Sergey Doroshenko
      9. ZOOKEEPER-784.patch
        60 kB
        Sergey Doroshenko
      10. ZOOKEEPER-784.patch
        36 kB
        Sergey Doroshenko
      11. ZOOKEEPER-784.patch
        21 kB
        Sergey Doroshenko
      12. ZOOKEEPER-784.patch
        14 kB
        Sergey Doroshenko
      13. ZOOKEEPER-784.patch
        12 kB
        Sergey Doroshenko

        Issue Links

          Activity

          Hide
          Sergey Doroshenko added a comment -

          Patch includes ReadOnlyZooKeeperServer and ReadOnlyReqestProcessor.
          This patch outlines the general idea: read-only server is spawned when peer becomes partitioned, and is able to serve read-only requests but drops state-changing ones.

          It's by no means a final version, however it's already possible to connect to a partitioned peer (or remain connected to it when it loses a quorum) and make read requests (write requests are dropped with explanation message).

          Next things to work on are:

          • session handling during partition. For now ReadOnlyServer creates SessionTrackerImpl and it's ok for manual tests, but that's just to make this proof-of-concept patch work
          • distinguish read-only clients and usual ones (depends on yet non-existent ticket about creation of read-only client), accept only read-onlies
          • create test cases, make all current ones pass
          Show
          Sergey Doroshenko added a comment - Patch includes ReadOnlyZooKeeperServer and ReadOnlyReqestProcessor. This patch outlines the general idea: read-only server is spawned when peer becomes partitioned, and is able to serve read-only requests but drops state-changing ones. It's by no means a final version, however it's already possible to connect to a partitioned peer (or remain connected to it when it loses a quorum) and make read requests (write requests are dropped with explanation message). Next things to work on are: session handling during partition. For now ReadOnlyServer creates SessionTrackerImpl and it's ok for manual tests, but that's just to make this proof-of-concept patch work distinguish read-only clients and usual ones (depends on yet non-existent ticket about creation of read-only client), accept only read-onlies create test cases, make all current ones pass
          Hide
          Sergey Doroshenko added a comment -

          Fixed JMX behaviour, now read-only server is visible in a JMX console when server is partitioned

          Show
          Sergey Doroshenko added a comment - Fixed JMX behaviour, now read-only server is visible in a JMX console when server is partitioned
          Hide
          Sergey Doroshenko added a comment -

          Added read-only functionality to client side: to create RO client, one have to pass additional boolean param to ZooKeeper's ctor; ctors with old signatures create usual clients.
          Now, if server is partitioned, it doesn't allow usual clients to connect.

          Decided not to create separate ticket for client-side change since it's anyway related to this one.

          Show
          Sergey Doroshenko added a comment - Added read-only functionality to client side: to create RO client, one have to pass additional boolean param to ZooKeeper's ctor; ctors with old signatures create usual clients. Now, if server is partitioned, it doesn't allow usual clients to connect. Decided not to create separate ticket for client-side change since it's anyway related to this one.
          Hide
          Patrick Hunt added a comment -

          I read through the wiki page, I think this is an awesome feature! A few high level questions came to mind:

          1) please be clear in the wiki page on b/w compatibility. There's some discussion of this, but what I mean is be very explicit regarding client->server compatibility, server->server compatibility. Have individual sections for this:

          a) new client new server (obv compat)
          b) new server new server (ditto)
          c) new client (using r/o client mode) old server
          d) new server -> old server

          2) be clear on how the client library will search for r/o server vs w/r server (if ensemble has internal partitioning)
          be clear on how this will work if no servers at all are available. In particular I'm interested in how the client will "backoff" vs hammering (constantly connecting/disconnecting to servers)

          3) add use cases that explain to users exactly how this will benefit them. go through some actual use cases such as leader election, group membership, and dynamic config. You should clearly spellout how the user's client code will handle these cases when using r/o mode. You might for example "rewrite" the recipes from the recipes page for the "r/o supported" version. Detail any special considerations the user might need to consider. For example, in the case of leader election you might detail how this is handled from zk perspective (client attached to r/o server) but also detail the considerations the user should have - such as the leader may have changed, what might the user have to consider. (leader can change if client A is connected to r/o server while the rest of the service (other clients of zk) might be connected to a r/w server, during which time the users's leadership changes). etc... We'll need this anyway for the user docs, doing it upfront on the jira will help you both for that and while developing the feature/tests.

          Show
          Patrick Hunt added a comment - I read through the wiki page, I think this is an awesome feature! A few high level questions came to mind: 1) please be clear in the wiki page on b/w compatibility. There's some discussion of this, but what I mean is be very explicit regarding client->server compatibility, server->server compatibility. Have individual sections for this: a) new client new server (obv compat) b) new server new server (ditto) c) new client (using r/o client mode) old server d) new server -> old server 2) be clear on how the client library will search for r/o server vs w/r server (if ensemble has internal partitioning) be clear on how this will work if no servers at all are available. In particular I'm interested in how the client will "backoff" vs hammering (constantly connecting/disconnecting to servers) 3) add use cases that explain to users exactly how this will benefit them. go through some actual use cases such as leader election, group membership, and dynamic config. You should clearly spellout how the user's client code will handle these cases when using r/o mode. You might for example "rewrite" the recipes from the recipes page for the "r/o supported" version. Detail any special considerations the user might need to consider. For example, in the case of leader election you might detail how this is handled from zk perspective (client attached to r/o server) but also detail the considerations the user should have - such as the leader may have changed, what might the user have to consider. (leader can change if client A is connected to r/o server while the rest of the service (other clients of zk) might be connected to a r/w server, during which time the users's leadership changes). etc... We'll need this anyway for the user docs, doing it upfront on the jira will help you both for that and while developing the feature/tests.
          Hide
          Patrick Hunt added a comment -

          For b/w compat I forgot: old client -> new server as well as new client not using the r/o feature against new/old server. I might have missed others...

          Show
          Patrick Hunt added a comment - For b/w compat I forgot: old client -> new server as well as new client not using the r/o feature against new/old server. I might have missed others...
          Hide
          Sergey Doroshenko added a comment -

          — Question about session handling —

          For now (latest patch) when r/o client connects to a partitioned server, new "fake" session is created for it – fake because it exists only in this server.
          When server regains the quorum, during this session's revalidation it will be rejected as invalid since leader didn't see this it.

          From users' point of view it'd be good to transparently upgrade such session to a usual session. The idea is that if leader sees that given session is invalid but also belongs to read-only client then it re-assigns new id to id and sends this new id to the client.

          The idea is good but seems error-prone.
          For example, what to do with r/o clients that were partitioned (and connected to partitioned server) for longer than sess timeout? At leader their sessions have already been expired, so they should be rejected. Re-assigning new session to such clients doesn't look right. But the problem here is that when servers see an invalid session they can't tell if they never saw it or if they saw it but it was expired.

          So, if quorum rejects r/o session (which implies this session either was never seen by quorum or is expired), there are two options from users' point of view:

          • ZooKeeper object becomes invalid, and application should create a new one. Reliable and consistent with current ZK
          • server transparently re-assigns session id for such client. Seems to have many potential problems, as described above

          What do you think?

          Show
          Sergey Doroshenko added a comment - — Question about session handling — For now (latest patch) when r/o client connects to a partitioned server, new "fake" session is created for it – fake because it exists only in this server. When server regains the quorum, during this session's revalidation it will be rejected as invalid since leader didn't see this it. From users' point of view it'd be good to transparently upgrade such session to a usual session. The idea is that if leader sees that given session is invalid but also belongs to read-only client then it re-assigns new id to id and sends this new id to the client. The idea is good but seems error-prone. For example, what to do with r/o clients that were partitioned (and connected to partitioned server) for longer than sess timeout? At leader their sessions have already been expired, so they should be rejected. Re-assigning new session to such clients doesn't look right. But the problem here is that when servers see an invalid session they can't tell if they never saw it or if they saw it but it was expired. So, if quorum rejects r/o session (which implies this session either was never seen by quorum or is expired), there are two options from users' point of view: ZooKeeper object becomes invalid, and application should create a new one. Reliable and consistent with current ZK server transparently re-assigns session id for such client. Seems to have many potential problems, as described above What do you think?
          Hide
          Henry Robinson added a comment -

          I like the idea of fake sessions fine, although I think that the upgrade process might be complex. Another possibility is to do away with sessions in read-only mode (because they're mainly used to maintain state about watches, which don't make sense on a read-only server).

          Sergey - just looked over your patch. Nice job! Couple of questions:

          1. In QuorumPeer.java, I can't quite follow the logic in this part of the patch:

          while (running) {
                           switch (getPeerState()) {
                           case LOOKING:
          +                    LOG.info("LOOKING");
          +                    ReadOnlyZooKeeperServer roZk = null;
                               try {
          -                        LOG.info("LOOKING");
          +                        roZk = new ReadOnlyZooKeeperServer(
          +                                    logFactory, this,
          +                                    new ZooKeeperServer.BasicDataTreeBuilder(),
          +                                    this.zkDb);
          +                        roZk.startup();
          +
          
          • is it sensible to start a ROZKServer every time a server enters the 'LOOKING' state, or should there be some kind of delay before it decides it is partitioned? Otherwise when a leader is lost and the quorum is doing a re-election, r/w clients that try and connect would get (I think) 'can't be read-only' messages .

          2. What are you doing about watches? It seems to me that setting a watch turns a read operation into a read / write operation, and the client should be told that watch registration failed. If you can do this you don't have to worry so much about session migration because there's very little session state maintained by a ROZKServer on behalf of the client.

          3. This patch has got to the point where it might be good if you started adding some tests to validate any further development you do.

          Show
          Henry Robinson added a comment - I like the idea of fake sessions fine, although I think that the upgrade process might be complex. Another possibility is to do away with sessions in read-only mode (because they're mainly used to maintain state about watches, which don't make sense on a read-only server). Sergey - just looked over your patch. Nice job! Couple of questions: 1. In QuorumPeer.java, I can't quite follow the logic in this part of the patch: while (running) { switch (getPeerState()) { case LOOKING: + LOG.info( "LOOKING" ); + ReadOnlyZooKeeperServer roZk = null ; try { - LOG.info( "LOOKING" ); + roZk = new ReadOnlyZooKeeperServer( + logFactory, this , + new ZooKeeperServer.BasicDataTreeBuilder(), + this .zkDb); + roZk.startup(); + is it sensible to start a ROZKServer every time a server enters the 'LOOKING' state, or should there be some kind of delay before it decides it is partitioned? Otherwise when a leader is lost and the quorum is doing a re-election, r/w clients that try and connect would get (I think) 'can't be read-only' messages . 2. What are you doing about watches? It seems to me that setting a watch turns a read operation into a read / write operation, and the client should be told that watch registration failed. If you can do this you don't have to worry so much about session migration because there's very little session state maintained by a ROZKServer on behalf of the client. 3. This patch has got to the point where it might be good if you started adding some tests to validate any further development you do.
          Hide
          Sergey Doroshenko added a comment -
          • As per Henry's suggestion, updated QuorumePeer to wait some time before starting RO server (currently it's 1 tickTime)
          • About watches: no, setting watches doesn't turn operation to a write operation.
            If a client is connected to a partitioned server and sets a watch in r/o mode, and meanwhile data is changed in the majority part, the watch will be triggered when the client reconnects to the majority. This is consistent with the current behavior, that is, if a watch is set before partitioning, it's triggered upon rejoining if there's any change
          • Added a test to ensure read operations succeed in r/o mode and write operations fail.
            Created QuorumUtil class which encapsulates quorum testing logic, e.g. allows to start/shutdown all peers or particular ones. It's used in the new test for r/o mode. I will refactor QuorumBase to be a special case of QuorumUtil.

          What I'll do next is update wiki page to address Patrick's comments and make r/o client to seek for r/w server (currently if client's connected to a r/o server it doesn't try to find another server).

          Show
          Sergey Doroshenko added a comment - As per Henry's suggestion, updated QuorumePeer to wait some time before starting RO server (currently it's 1 tickTime) About watches: no, setting watches doesn't turn operation to a write operation. If a client is connected to a partitioned server and sets a watch in r/o mode, and meanwhile data is changed in the majority part, the watch will be triggered when the client reconnects to the majority. This is consistent with the current behavior, that is, if a watch is set before partitioning, it's triggered upon rejoining if there's any change Added a test to ensure read operations succeed in r/o mode and write operations fail. Created QuorumUtil class which encapsulates quorum testing logic, e.g. allows to start/shutdown all peers or particular ones. It's used in the new test for r/o mode. I will refactor QuorumBase to be a special case of QuorumUtil. What I'll do next is update wiki page to address Patrick's comments and make r/o client to seek for r/w server (currently if client's connected to a r/o server it doesn't try to find another server).
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 10 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 generated 20 release audit warnings (more than the trunk's current 17 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-h1.grid.sp2.yahoo.net/125/testReport/
          Release audit warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/125/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/125/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/125/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12448163/ZOOKEEPER-784.patch against trunk revision 958096. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 10 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 generated 20 release audit warnings (more than the trunk's current 17 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-h1.grid.sp2.yahoo.net/125/testReport/ Release audit warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/125/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/125/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/125/console This message is automatically generated.
          Hide
          Sergey Doroshenko added a comment -

          In this patch:

          • client seeks for r/w server if it's connected to r/o server, backoffing exponentially.
          • "fake" session (the one assigned by r/o server) is transitioned to valid session upon connection to r/w server, transparently for the user
          • client receives notifications about r/o mode via default watcher
          • more tests
          Show
          Sergey Doroshenko added a comment - In this patch: client seeks for r/w server if it's connected to r/o server, backoffing exponentially. "fake" session (the one assigned by r/o server) is transitioned to valid session upon connection to r/w server, transparently for the user client receives notifications about r/o mode via default watcher more tests
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 13 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 1 new Findbugs warnings.

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

          -1 core tests. The patch failed core unit tests.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12448584/ZOOKEEPER-784.patch against trunk revision 958096. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 13 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 1 new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/130/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/130/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/130/console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

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

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

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

          -1 patch. The patch command could not apply the patch.

          Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/132/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12448721/ZOOKEEPER-784.patch against trunk revision 960676. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 13 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/132/console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 13 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 failed core unit tests.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12448725/ZOOKEEPER-784.patch against trunk revision 960686. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 13 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 failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/93/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/93/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/93/console This message is automatically generated.
          Hide
          Sergey Doroshenko added a comment -

          I've updated the wiki page (http://wiki.apache.org/hadoop/ZooKeeper/GSoCReadOnlyMode) to address backwards compatibility, searching of r/w server during r-o mode, session handling and other questions, and made it more structured overall. Soon I will also add some use case / recipe updated with regard to read-only mode.

          Overall, the latest patch presents ready-to-use implementation of the read-only mode feature. It's fully backwards compatible, new server is safe to use along with old ones, new client is safe to use against old servers etc.
          Documentation and tests will be improved of course, but again, this patch is ready to use. You could try this if you're interested. Consult the wiki page about usage and design details.

          Show
          Sergey Doroshenko added a comment - I've updated the wiki page ( http://wiki.apache.org/hadoop/ZooKeeper/GSoCReadOnlyMode ) to address backwards compatibility, searching of r/w server during r-o mode, session handling and other questions, and made it more structured overall. Soon I will also add some use case / recipe updated with regard to read-only mode. Overall, the latest patch presents ready-to-use implementation of the read-only mode feature. It's fully backwards compatible, new server is safe to use along with old ones, new client is safe to use against old servers etc. Documentation and tests will be improved of course, but again, this patch is ready to use. You could try this if you're interested. Consult the wiki page about usage and design details.
          Hide
          Sergey Doroshenko added a comment -

          Fixed issue with compatibility with C client. Now all tests should pass

          Show
          Sergey Doroshenko added a comment - Fixed issue with compatibility with C client. Now all tests should pass
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 13 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 failed core unit tests.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12449071/ZOOKEEPER-784.patch against trunk revision 961026. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 13 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 failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/136/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/136/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/136/console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 13 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 failed core unit tests.

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

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12449071/ZOOKEEPER-784.patch against trunk revision 961026. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 13 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 failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/137/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/137/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/137/console This message is automatically generated.
          Hide
          Sergey Doroshenko added a comment -

          Hudson says there's one failing test: QuorumZxidSyncTest.testBehindLeader. But that's strange.
          1) I can't reproduce this on my machine – all tests pass.
          2) Also, this test was passing ok with previous patch. And current patch is nearly the same as the previous one, changes made after previous patch can't cause this test to fail.

          Show
          Sergey Doroshenko added a comment - Hudson says there's one failing test: QuorumZxidSyncTest.testBehindLeader. But that's strange. 1) I can't reproduce this on my machine – all tests pass. 2) Also, this test was passing ok with previous patch. And current patch is nearly the same as the previous one, changes made after previous patch can't cause this test to fail.
          Hide
          Sergey Doroshenko added a comment -

          updated the patch wrt latest trunk's state

          Show
          Sergey Doroshenko added a comment - updated the patch wrt latest trunk's state
          Hide
          Henry Robinson added a comment -

          Sergey this looks great - thanks! I'll take a look at it asap.

          Show
          Henry Robinson added a comment - Sergey this looks great - thanks! I'll take a look at it asap.
          Hide
          Sergey Doroshenko added a comment -

          improved documentation

          Show
          Sergey Doroshenko added a comment - improved documentation
          Hide
          Henry Robinson added a comment -

          Spectacular job, Sergey. I've taken a look at the code and I'm pretty satisfied - you've done a great job covering little things like JMX support, and good code comments and documentation.

          I'm going to wait for one of the other committers to come by and also give this a +1 since this is a substantial change. We may also decide to run a long-lived test with this patch to satisfy ourselves of the stability. But this looks very, very solid indeed.

          Show
          Henry Robinson added a comment - Spectacular job, Sergey. I've taken a look at the code and I'm pretty satisfied - you've done a great job covering little things like JMX support, and good code comments and documentation. I'm going to wait for one of the other committers to come by and also give this a +1 since this is a substantial change. We may also decide to run a long-lived test with this patch to satisfy ourselves of the stability. But this looks very, very solid indeed.
          Hide
          Sergey Doroshenko added a comment -

          Thanks Henry! Eagerly looking forward to another +1 and getting r-o mode into the trunk

          I also have a patch that applies above ZOOKEEPER-733 changes, so if those go to the trunk first, I'll upload the patch

          Show
          Sergey Doroshenko added a comment - Thanks Henry! Eagerly looking forward to another +1 and getting r-o mode into the trunk I also have a patch that applies above ZOOKEEPER-733 changes, so if those go to the trunk first, I'll upload the patch
          Hide
          Sergey Doroshenko added a comment -

          The patch against latest trunk state

          Show
          Sergey Doroshenko added a comment - The patch against latest trunk state
          Hide
          Flavio Junqueira added a comment -

          Nice job, Sergey! It is pretty well done, so I just have a few comments:

          1. It is worth adding a warn message to ZooKeeperServer to log when an old client connects to an ro-enabled server. Currently the catch block doesn't do anything;
          2. The code of ReadOnlyRequestProcessor is straightforward, but it would be nice to add a high-level description to the beginning of the class (add javadocs in general);
          3. I wonder if waiting to start the r-o server for a single tickTime is not too aggressive. Would it make sense to set a lower bound of say 2s? That is, we use the largest between tickTime and 2s. An alternative is to add an optional parameter to set this value;
          4. I couldn't really understand how the r-o zookeeper server gets killed once the replica joins a working quorum (i.e., once it transitions to either FOLLOWING, LEADING, or OBSERVING). I understand that leader election keeps running regularly, and the ro server starts if the state of the server doesn't change within the timeout period, but I couldn't find the code that kills the r-o server in the case it starts and the replica is not LOOKING. Could you please point me out to code that does it?
          Show
          Flavio Junqueira added a comment - Nice job, Sergey! It is pretty well done, so I just have a few comments: It is worth adding a warn message to ZooKeeperServer to log when an old client connects to an ro-enabled server. Currently the catch block doesn't do anything; The code of ReadOnlyRequestProcessor is straightforward, but it would be nice to add a high-level description to the beginning of the class (add javadocs in general); I wonder if waiting to start the r-o server for a single tickTime is not too aggressive. Would it make sense to set a lower bound of say 2s? That is, we use the largest between tickTime and 2s. An alternative is to add an optional parameter to set this value; I couldn't really understand how the r-o zookeeper server gets killed once the replica joins a working quorum (i.e., once it transitions to either FOLLOWING, LEADING, or OBSERVING). I understand that leader election keeps running regularly, and the ro server starts if the state of the server doesn't change within the timeout period, but I couldn't find the code that kills the r-o server in the case it starts and the replica is not LOOKING. Could you please point me out to code that does it?
          Hide
          Sergey Doroshenko added a comment -

          Hey Flavio, thanks for reviewing!

          Re your comments:
          1. added warn messages both to server- and client-side
          2. done
          3. done
          4. r-o server gets killed in "finally" block of LOOKING case of QuorumPeer#run (QuorumPeer#645 in the latest patch). Interesting thing is, it may actually won't have started by the time shutdown is called (if peer regains the quorum during the grace period), but #shutdown is called in any case

          Show
          Sergey Doroshenko added a comment - Hey Flavio, thanks for reviewing! Re your comments: 1. added warn messages both to server- and client-side 2. done 3. done 4. r-o server gets killed in "finally" block of LOOKING case of QuorumPeer#run (QuorumPeer#645 in the latest patch). Interesting thing is, it may actually won't have started by the time shutdown is called (if peer regains the quorum during the grace period), but #shutdown is called in any case
          Hide
          Flavio Junqueira added a comment -

          +1, great job, Sergey! And thanks for the clarification, I can't believe I missed that finally block.

          Show
          Flavio Junqueira added a comment - +1, great job, Sergey! And thanks for the clarification, I can't believe I missed that finally block.
          Hide
          Hadoop QA added a comment -

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

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

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

          -1 patch. The patch command could not apply the patch.

          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/12//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12452718/ZOOKEEPER-784.patch against trunk revision 1033155. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/12//console This message is automatically generated.
          Hide
          Patrick Hunt added a comment -

          Looks like the patch is failing to apply in one hunk, please resubmit. thanks.

          [exec] 1 out of 2 hunks FAILED – saving rejects to file src/java/main/org/apache/zookeeper/Watcher.java.rej

          Show
          Patrick Hunt added a comment - Looks like the patch is failing to apply in one hunk, please resubmit. thanks. [exec] 1 out of 2 hunks FAILED – saving rejects to file src/java/main/org/apache/zookeeper/Watcher.java.rej
          Hide
          Sergey Doroshenko added a comment -

          Hi Patrick,

          Currently I'm in the final stage of my Facebook's internship, and I want to keep good pace and not be distracted by other things.
          Hopefully r-o mode is not release-blocking feature, so I hope it'll be ok if I resubmit the patch in 3 weeks. But if this should be done sooner, let me know and I'll try to find a time

          Show
          Sergey Doroshenko added a comment - Hi Patrick, Currently I'm in the final stage of my Facebook's internship, and I want to keep good pace and not be distracted by other things. Hopefully r-o mode is not release-blocking feature, so I hope it'll be ok if I resubmit the patch in 3 weeks. But if this should be done sooner, let me know and I'll try to find a time
          Hide
          Patrick Hunt added a comment -

          No worries, I'd like to get this in given you've done a bunch of work on it, qabot just flagged it given it's recently working again. thanks.

          Show
          Patrick Hunt added a comment - No worries, I'd like to get this in given you've done a bunch of work on it, qabot just flagged it given it's recently working again. thanks.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 11 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 9 new Findbugs (version 1.3.9) 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: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/110//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/110//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/110//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12470000/ZOOKEEPER-784.patch against trunk revision 1062244. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 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 9 new Findbugs (version 1.3.9) 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: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/110//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/110//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/110//console This message is automatically generated.
          Hide
          Sergey Doroshenko added a comment -

          minor update in StatCommand

          Show
          Sergey Doroshenko added a comment - minor update in StatCommand
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 11 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 9 new Findbugs (version 1.3.9) 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: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/118//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/118//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/118//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12470236/ZOOKEEPER-784.patch against trunk revision 1062244. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 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 9 new Findbugs (version 1.3.9) 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: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/118//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/118//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/118//console This message is automatically generated.
          Hide
          Sergey Doroshenko added a comment -

          Though this patch already has two +1s from Henry and Flavio, I suppose someone could review it (and ZOOKEEPER-827) one more time.

          Overall logic remained the same; changes were mainly due to recent client-side code refactorings, e.g. "move code from ClientCnxn to ClientCnxnSocket".

          Findbugs warnings has nothing to do with this patch, so I hope it will go into trunk some time soon.

          Show
          Sergey Doroshenko added a comment - Though this patch already has two +1s from Henry and Flavio, I suppose someone could review it (and ZOOKEEPER-827 ) one more time. Overall logic remained the same; changes were mainly due to recent client-side code refactorings, e.g. "move code from ClientCnxn to ClientCnxnSocket". Findbugs warnings has nothing to do with this patch, so I hope it will go into trunk some time soon.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 11 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 (version 1.3.9) 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: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/150//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/150//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/150//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12470236/ZOOKEEPER-784.patch against trunk revision 1072085. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 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 (version 1.3.9) 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: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/150//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/150//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/150//console This message is automatically generated.
          Hide
          Sergey Doroshenko added a comment -

          Has anybody had a chance to look at the latest patch?
          It's already approved by Hudson, and its previous version has two +1s, so I hope it'll get into trunk soon.

          Show
          Sergey Doroshenko added a comment - Has anybody had a chance to look at the latest patch? It's already approved by Hudson, and its previous version has two +1s, so I hope it'll get into trunk soon.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 11 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 (version 1.3.9) warnings.

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

          -1 core tests. The patch failed core unit tests.

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

          Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/169//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/169//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/169//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12470236/ZOOKEEPER-784.patch against trunk revision 1072085. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 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 (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/169//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/169//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/169//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

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

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

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

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

          -1 javac. The patch appears to cause tar ant target to fail.

          -1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.

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

          -1 core tests. The patch failed core unit tests.

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

          Test results: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/238//testReport/
          Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/238//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12470236/ZOOKEEPER-784.patch against trunk revision 1099267. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The patch appears to cause tar ant target to fail. -1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/238//testReport/ Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/238//console This message is automatically generated.
          Hide
          Ketan G added a comment -

          I have updated Sergey's patch to:

          • apply to trunk
          • incorporate one more change he made to StatCommand in NettyServerCnxn.java
          • change log4j references to slf4j

          I have successfully run ant releaseaudit on the result.

          Show
          Ketan G added a comment - I have updated Sergey's patch to: apply to trunk incorporate one more change he made to StatCommand in NettyServerCnxn.java change log4j references to slf4j I have successfully run ant releaseaudit on the result.
          Hide
          Hadoop QA added a comment -

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

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

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

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/241//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12478116/ZOOKEEPER-784.patch against trunk revision 1099267. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/241//console This message is automatically generated.
          Hide
          Ketan G added a comment -

          Previous patch was malformed.

          Show
          Ketan G added a comment - Previous patch was malformed.
          Hide
          Hadoop QA added a comment -

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

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

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

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/246//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12478172/ZOOKEEPER-784.patch against trunk revision 1099329. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/246//console This message is automatically generated.
          Hide
          Ketan G added a comment -

          Trying again.

          Show
          Ketan G added a comment - Trying again.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 11 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 (version 1.3.9) warnings.

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

          -1 core tests. The patch failed core unit tests.

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

          Test results: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/247//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/247//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/247//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12478176/ZOOKEEPER-784.patch against trunk revision 1099329. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 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 (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/247//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/247//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/247//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 11 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 (version 1.3.9) 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: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/277//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/277//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/277//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12478176/ZOOKEEPER-784.patch against trunk revision 1103811. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 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 (version 1.3.9) 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: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/277//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/277//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/277//console This message is automatically generated.
          Hide
          Henry Robinson added a comment -

          I just committed this. Thanks Sergey, and everyone who contributed!

          Show
          Henry Robinson added a comment - I just committed this. Thanks Sergey, and everyone who contributed!
          Hide
          Patrick Hunt added a comment -

          Great to see all the work on this one, thanks everyone and esp Sergey!

          Show
          Patrick Hunt added a comment - Great to see all the work on this one, thanks everyone and esp Sergey!
          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #1188 (See https://builds.apache.org/hudson/job/ZooKeeper-trunk/1188/)
          ZOOKEEPER-784. Server-side functionality for read-only mode (Sergey Doroshenko via henryr) - add missing files
          ZOOKEEPER-784. Server-side functionality for read-only mode (Sergey Doroshenko via henryr)

          henry : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1125179
          Files :

          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyBean.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java

          henry : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1125176
          Files :

          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeperMain.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ClientBase.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/KeeperException.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/Watcher.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/QuorumBase.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxn.java
          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxnSocket.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #1188 (See https://builds.apache.org/hudson/job/ZooKeeper-trunk/1188/ ) ZOOKEEPER-784 . Server-side functionality for read-only mode (Sergey Doroshenko via henryr) - add missing files ZOOKEEPER-784 . Server-side functionality for read-only mode (Sergey Doroshenko via henryr) henry : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1125179 Files : /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyBean.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ReadOnlyModeTest.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java henry : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1125176 Files : /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeperMain.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ClientBase.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/KeeperException.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/Watcher.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/QuorumBase.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxn.java /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxnSocket.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
          Hide
          Benjamin Reed added a comment -

          how does this affect legacy code. will everyone start seeing the message that they are switching into readonly?

          Show
          Benjamin Reed added a comment - how does this affect legacy code. will everyone start seeing the message that they are switching into readonly?
          Hide
          Sergey Doroshenko added a comment -

          Awesome to see this is finally integrated! Thanks Henry for reviewing it again and Ketan for contributing.

          @Benjamin: this change is fully backwards compatible. A client will be able to connect to read-only server only if it (client) has 'canBeReadOnly' flag (which is off by default) explicitly set. Old clients won't notice anything, they'll still receive an exception saying there's no quorum. You can check out http://wiki.apache.org/hadoop/ZooKeeper/GSoCReadOnlyMode, the design is outlined there.

          Show
          Sergey Doroshenko added a comment - Awesome to see this is finally integrated! Thanks Henry for reviewing it again and Ketan for contributing. @Benjamin: this change is fully backwards compatible. A client will be able to connect to read-only server only if it (client) has 'canBeReadOnly' flag (which is off by default) explicitly set. Old clients won't notice anything, they'll still receive an exception saying there's no quorum. You can check out http://wiki.apache.org/hadoop/ZooKeeper/GSoCReadOnlyMode , the design is outlined there.
          Hide
          Benjamin Reed added a comment -

          great. awesome job sergey!

          Show
          Benjamin Reed added a comment - great. awesome job sergey!

            People

            • Assignee:
              Sergey Doroshenko
              Reporter:
              Sergey Doroshenko
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development