You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Skye Wanderman-Milne <sk...@cloudera.com> on 2013/01/04 03:17:06 UTC

Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/
-----------------------------------------------------------

(Updated Jan. 4, 2013, 2:17 a.m.)


Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.


Changes
-------

Rebased on trunk again (Unfortunately this required some pretty large changes, making the diff between this patch and the previous one hard to read.)

Made it possible to disable the AdminServer either by setting the zookeeper.admin.enableAdminServer system property to false or by removing jetty from the classpath (in case users don't want to depend on Jetty). I implemented this by extracting AdminServer into an interface with two subclasses, JettyAdminServer (the original implementation) and DummyAdminServer (which does nothing and is used when the server is disabled). AdminServerFactory is then responsible for creating the appropriate server. I updated the documentation to reflect this.

Using the system property, I disabled the AdminServer during tests as it was causing some tests to hang when they tried to start multiple AdminServers on the same port. I also added some more comments and renamed some functions to make them clearer.


Description
-------

See my comment in ZOOKEEPER-1346.


This addresses bug ZOOKEEPER-1346.
    https://issues.apache.org/jira/browse/ZOOKEEPER-1346


Diffs (updated)
-----

  ivy.xml fadf4f4 
  src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
  src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
  src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
  src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
  src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
  src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
  src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
  src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
  src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
  src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
  src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
  src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
  src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
  src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
  src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
  src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
  src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
  src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
  src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
  src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
  src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 

Diff: https://reviews.apache.org/r/8094/diff/


Testing
-------

unit tests

Ran in standalone mode (only option right now) and manually tried out all the commands/links


Thanks,

Skye Wanderman-Milne


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Skye Wanderman-Milne <sk...@cloudera.com>.

> On Jan. 16, 2013, 1:51 a.m., Edward Ribeiro wrote:
> > src/java/main/org/apache/zookeeper/server/admin/Commands.java, line 328
> > <https://reviews.apache.org/r/8094/diff/4/?file=244397#file244397line328>
> >
> >     We may replace this line by:
> >     
> >     if (stats.getServerState().equals("leader") && zkServer instanceof LeaderZooKeeperServer) {
> >     
> >     so that you can get rid of the FindBugs message (2nd one).

I'll just change it to "if (zkServer instanceof LeaderZooKeeperServer)" (the two conditions should be equivalent...)


- Skye


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15373
-----------------------------------------------------------


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15373
-----------------------------------------------------------



src/java/main/org/apache/zookeeper/server/admin/Commands.java
<https://reviews.apache.org/r/8094/#comment33062>

    We may replace this line by:
    
    if (stats.getServerState().equals("leader") && zkServer instanceof LeaderZooKeeperServer) {
    
    so that you can get rid of the FindBugs message (2nd one).


- Edward Ribeiro


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15378
-----------------------------------------------------------



src/java/main/org/apache/zookeeper/server/DataTree.java
<https://reviews.apache.org/r/8094/#comment33068>

    Replace "that that" by "whose"


- Edward Ribeiro


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15374
-----------------------------------------------------------



src/java/main/org/apache/zookeeper/server/DataTree.java
<https://reviews.apache.org/r/8094/#comment33063>

    You may use ephemerals.entrySet() instead of ephemerals.keySet() here and proceed accordingly.


- Edward Ribeiro


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Skye Wanderman-Milne <sk...@cloudera.com>.

> On Jan. 9, 2013, 7:13 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/ExpiryQueue.java, line 181
> > <https://reviews.apache.org/r/8094/diff/4/?file=244379#file244379line181>
> >
> >     is it necessary to break encapsulation like this? would be safer if we didn't.
> >     
> >     worst case we could wrap in http://docs.oracle.com/javase/6/docs/api/java/util/Collections.html#unmodifiableMap(java.util.Map) but even then the value would still be modifyable...

I'm going with the unmodifiableMap. The value is still modifiable, but this is unavoidable since the ExpiryQueue doesn't know anything about the values it's storing -- no matter how you slice it, SessionTrackerImpl needs access to the stored SessionImpl values so it can extract the IDs, at least without ExpiryMap getting super fancy. I think exposing the values is pretty reasonable, though, since ExpiryQueue doesn't depend on not modifying the stored values and the values are provided by outside callers to begin with. I agree it's ugly but I think this is the simplest and best solution.


- Skye


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15204
-----------------------------------------------------------


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Skye Wanderman-Milne <sk...@cloudera.com>.

> On Jan. 9, 2013, 7:13 p.m., Patrick Hunt wrote:
> > src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml, line 1210
> > <https://reviews.apache.org/r/8094/diff/4/?file=244377#file244377line1210>
> >
> >     is there a configuration option to disable this feature? I don't see any doc details on this...
> >     
> >     admin.enableAdminServer  seems to be missing?

I somehow managed to document admin.enableAdminServer in my latest patch to the JIRA but not here... regardless it's there now.


> On Jan. 9, 2013, 7:13 p.m., Patrick Hunt wrote:
> > src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml, line 1211
> > <https://reviews.apache.org/r/8094/diff/4/?file=244377#file244377line1211>
> >
> >     this should be "New in 3.5.0", won't get into a fix release.

Done.


> On Jan. 9, 2013, 7:13 p.m., Patrick Hunt wrote:
> > src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml, line 1234
> > <https://reviews.apache.org/r/8094/diff/4/?file=244377#file244377line1234>
> >
> >     isn't "/" root? Not sure what you mean by "for now nothing is served directly from the root"

Hmm not sure if root is or isn't the right term here.

I mean right now everything happens at "/commands" (by default) and "/" is a 404. I'm tempted to remove this option altogether and only have the command URL option for simplicity.


- Skye


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15204
-----------------------------------------------------------


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Skye Wanderman-Milne <sk...@cloudera.com>.

> On Jan. 9, 2013, 7:13 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 980
> > <https://reviews.apache.org/r/8094/diff/4/?file=244389#file244389line980>
> >
> >     same issue here wrt exposing the map, safety.

This one should be ok since getSessionExpiryMap creates the returned Map on the fly -- it's safe to mutate it since there's no other reference to it.


- Skye


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15204
-----------------------------------------------------------


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Patrick Hunt <ph...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15204
-----------------------------------------------------------



src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
<https://reviews.apache.org/r/8094/#comment32798>

    is there a configuration option to disable this feature? I don't see any doc details on this...
    
    admin.enableAdminServer  seems to be missing?



src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
<https://reviews.apache.org/r/8094/#comment32792>

    this should be "New in 3.5.0", won't get into a fix release.



src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
<https://reviews.apache.org/r/8094/#comment32794>

    isn't "/" root? Not sure what you mean by "for now nothing is served directly from the root"



src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
<https://reviews.apache.org/r/8094/#comment32795>

    3.5.0



src/java/main/org/apache/zookeeper/server/ExpiryQueue.java
<https://reviews.apache.org/r/8094/#comment32800>

    is it necessary to break encapsulation like this? would be safer if we didn't.
    
    worst case we could wrap in http://docs.oracle.com/javase/6/docs/api/java/util/Collections.html#unmodifiableMap(java.util.Map) but even then the value would still be modifyable...



src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
<https://reviews.apache.org/r/8094/#comment32803>

    same issue here wrt exposing the map, safety.


- Patrick Hunt


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15379
-----------------------------------------------------------



src/java/main/org/apache/zookeeper/server/WatchManager.java
<https://reviews.apache.org/r/8094/#comment33075>

    I guess lines 180-187 could be rewritten as:
    
    for (Map.Entry<Watcher,Set<String>> e: watch2Paths.entrySet()) {
           Long id = ((ServerCnxn) e.getKey()).getSessionId();
           HashSet<String> paths = new HashSet<String>(e.getValue());
           id2paths.put(id, paths);
    
    }
    
    


- Edward Ribeiro


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15457
-----------------------------------------------------------



src/java/main/org/apache/zookeeper/server/admin/Commands.java
<https://reviews.apache.org/r/8094/#comment33337>

    Would you mind to make this lines 57 and 58  "final" too?
    
    Just a suggestion, but otherwise I think that this patch is ready to ship. Congratulations on the good work. :)


- Edward Ribeiro


On Jan. 17, 2013, 2:01 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2013, 2:01 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   docs/zookeeperAdmin.html bc2b331 
>   docs/zookeeperAdmin.pdf ed595d2 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 87ebd7b 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/admin/JettyAdminServerTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 2707c26 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java c39f4c9 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java f107e04 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Edward Ribeiro <ed...@gmail.com>.

> On Jan. 17, 2013, 11:53 p.m., Edward Ribeiro wrote:
> > src/java/test/org/apache/zookeeper/server/admin/JettyAdminServerTest.java, line 1
> > <https://reviews.apache.org/r/8094/diff/5/?file=249522#file249522line1>
> >
> >     Add Apache license (Jenkins alert).

Skye has just added the license. It's okay to close this review now.


- Edward


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15475
-----------------------------------------------------------


On Jan. 17, 2013, 2:01 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2013, 2:01 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   docs/zookeeperAdmin.html bc2b331 
>   docs/zookeeperAdmin.pdf ed595d2 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 87ebd7b 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/admin/JettyAdminServerTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 2707c26 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java c39f4c9 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java f107e04 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15475
-----------------------------------------------------------



src/java/test/org/apache/zookeeper/server/admin/JettyAdminServerTest.java
<https://reviews.apache.org/r/8094/#comment33411>

    Add Apache license (Jenkins alert).


- Edward Ribeiro


On Jan. 17, 2013, 2:01 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2013, 2:01 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   docs/zookeeperAdmin.html bc2b331 
>   docs/zookeeperAdmin.pdf ed595d2 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 87ebd7b 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/admin/JettyAdminServerTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 2707c26 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java c39f4c9 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java f107e04 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Skye Wanderman-Milne <sk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/
-----------------------------------------------------------

(Updated Jan. 17, 2013, 2:01 a.m.)


Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.


Changes
-------

Added JettyAdminServerTest. This required some changes to make it work such as shutting down the Jetty server to make sure it released it port between tests and changing where the admin system properties are read so they are refreshed when a new server is created.

Rebased on trunk and addressed review comments.


Description
-------

See my comment in ZOOKEEPER-1346.


This addresses bug ZOOKEEPER-1346.
    https://issues.apache.org/jira/browse/ZOOKEEPER-1346


Diffs (updated)
-----

  docs/zookeeperAdmin.html bc2b331 
  docs/zookeeperAdmin.pdf ed595d2 
  ivy.xml fadf4f4 
  src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
  src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
  src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
  src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
  src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
  src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
  src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
  src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
  src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
  src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
  src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
  src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
  src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
  src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/quorum/Leader.java 87ebd7b 
  src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
  src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
  src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
  src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
  src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
  src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
  src/java/test/org/apache/zookeeper/server/admin/JettyAdminServerTest.java PRE-CREATION 
  src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 2707c26 
  src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java c39f4c9 
  src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java f107e04 

Diff: https://reviews.apache.org/r/8094/diff/


Testing
-------

unit tests

Ran in standalone mode (only option right now) and manually tried out all the commands/links


Thanks,

Skye Wanderman-Milne


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15375
-----------------------------------------------------------



src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java
<https://reviews.apache.org/r/8094/#comment33064>

    What about modifying lines 115-124 by these below: 
    
            Map<Long, Set<Long>> sessionExpiryMap = new TreeMap<Long, Set<Long>>();
            for (Map.Entry<long,Set<Long>> e : expiryMap.entrySet()) {
    	    long time = e.getKey();
                Set<Long> ids = new HashSet<Long>(e.getValue());
                sessionExpiryMap.put(time, ids);
            }
    
    Same behavior, less code. But it's just a suggestion, so please feel free to drop it (or point any error/misunderstanding of mine).


- Edward Ribeiro


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Skye Wanderman-Milne <sk...@cloudera.com>.

> On Jan. 9, 2013, 7:15 p.m., Patrick Hunt wrote:
> > Skye you mentioned "Using the system property, I disabled the AdminServer during tests as it was causing some tests to hang when they tried to start multiple AdminServers on the same port"
> > 
> > I don't see a test that's not disabling the AdminServer - do you have a test to verify the AdminServer itself? (rather than the individual commands)
> >

I didn't, I added JettyAdminServerTest which tests that an admin server is started for a quorum and for a standalone server.


- Skye


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15205
-----------------------------------------------------------


On Jan. 17, 2013, 2:01 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2013, 2:01 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   docs/zookeeperAdmin.html bc2b331 
>   docs/zookeeperAdmin.pdf ed595d2 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 87ebd7b 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/admin/JettyAdminServerTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 2707c26 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java c39f4c9 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java f107e04 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Patrick Hunt <ph...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15205
-----------------------------------------------------------


Skye you mentioned "Using the system property, I disabled the AdminServer during tests as it was causing some tests to hang when they tried to start multiple AdminServers on the same port"

I don't see a test that's not disabling the AdminServer - do you have a test to verify the AdminServer itself? (rather than the individual commands)


- Patrick Hunt


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Skye Wanderman-Milne <sk...@cloudera.com>.

> On Jan. 16, 2013, 2:50 a.m., Edward Ribeiro wrote:
> > src/java/main/org/apache/zookeeper/server/WatchManager.java, line 177
> > <https://reviews.apache.org/r/8094/diff/4/?file=244387#file244387line177>
> >
> >     Replace "that that" by "whose".

"that that" -> "which that" (read it in context, it makes sense I swear)


- Skye


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15377
-----------------------------------------------------------


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Skye Wanderman-Milne <sk...@cloudera.com>.

> On Jan. 16, 2013, 2:50 a.m., Edward Ribeiro wrote:
> > src/java/main/org/apache/zookeeper/server/WatchManager.java, line 177
> > <https://reviews.apache.org/r/8094/diff/4/?file=244387#file244387line177>
> >
> >     Replace "that that" by "whose".
> 
> Skye Wanderman-Milne wrote:
>     "that that" -> "which that" (read it in context, it makes sense I swear)

Actually "that the"


- Skye


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15377
-----------------------------------------------------------


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15377
-----------------------------------------------------------



src/java/main/org/apache/zookeeper/server/WatchManager.java
<https://reviews.apache.org/r/8094/#comment33066>

    Replace "that that" by "whose".


- Edward Ribeiro


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>


Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review15380
-----------------------------------------------------------



src/java/main/org/apache/zookeeper/server/WatchManager.java
<https://reviews.apache.org/r/8094/#comment33076>

    What about rewrite the lines 196-202 to:
    
      Map<String, Set<Long>> path2ids = new HashMap<String, Set<Long>>();	
      for (Map.Entry<String,Set<Long>> e : watchTable.entrySet()) {
          String path = e.getKey();
          Set<Long> ids = new HashSet<Long>(e.getValue().size());
          path2ids.put(path, ids);
          for (Watcher watcher : e.getValue()) {
              ids.add(((ServerCnxn) watcher).getSessionId());
          }
      }
    
    It's not a must to rewrite, but the creation of "ids" variable would be very cool. Just a suggestion tough (as everything I wrote before). Please, compare these suggestions with your original code to see if it's correct *and* worth rewrite.


- Edward Ribeiro


On Jan. 4, 2013, 2:17 a.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2013, 2:17 a.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
>   src/java/main/org/apache/zookeeper/server/DataTree.java d6c7773 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 9422538 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 267dbdf 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 137862e 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 14e754b 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/DummyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 8a432ff 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 9c17f5e 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java deae926 
>   src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java d3f1492 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 8665bac 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 370e7bb 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>