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

[Edward Ribeiro]

> On Jan. 17, 2013, 11:53 p.m., Edward Ribeiro wrote:
> > src/java/test/org/apache/zookeeper/server/admin/JettyAdminServerTest.java, 
> > line 1
> > 
> >
> > 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)

[Edward Ribeiro]

---
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


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)

[Edward Ribeiro]

---
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


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)

[Skye Wanderman-Milne]

> 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)

[Skye Wanderman-Milne]

---
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)

[Skye Wanderman-Milne]

> On Jan. 16, 2013, 2:50 a.m., Edward Ribeiro wrote:
> > src/java/main/org/apache/zookeeper/server/WatchManager.java, line 177
> > 
> >
> > 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)

[Skye Wanderman-Milne]

> On Jan. 16, 2013, 2:50 a.m., Edward Ribeiro wrote:
> > src/java/main/org/apache/zookeeper/server/WatchManager.java, line 177
> > 
> >
> > 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)

[Skye Wanderman-Milne]

> On Jan. 16, 2013, 1:51 a.m., Edward Ribeiro wrote:
> > src/java/main/org/apache/zookeeper/server/admin/Commands.java, line 328
> > 
> >
> > 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)

[Edward Ribeiro]

---
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


What about rewrite the lines 196-202 to:

  Map> path2ids = new HashMap>();   
  for (Map.Entry> e : watchTable.entrySet()) {
  String path = e.getKey();
  Set ids = new HashSet(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
> 
>

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

[Edward Ribeiro]

---
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


I guess lines 180-187 could be rewritten as:

for (Map.Entry> e: watch2Paths.entrySet()) {
   Long id = ((ServerCnxn) e.getKey()).getSessionId();
   HashSet paths = new HashSet(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)

[Edward Ribeiro]

---
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


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)

[Edward Ribeiro]

---
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


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)

[Edward Ribeiro]

---
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


What about modifying lines 115-124 by these below: 

Map> sessionExpiryMap = new TreeMap>();
for (Map.Entry> e : expiryMap.entrySet()) {
long time = e.getKey();
Set ids = new HashSet(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)

[Edward Ribeiro]

---
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


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)

[Edward Ribeiro]

---
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


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)

[Skye Wanderman-Milne]

> On Jan. 9, 2013, 7:13 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/ExpiryQueue.java, line 181
> > 
> >
> > 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)

[Skye Wanderman-Milne]

> On Jan. 9, 2013, 7:13 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java, line 980
> > 
> >
> > 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)

[Skye Wanderman-Milne]

> On Jan. 9, 2013, 7:13 p.m., Patrick Hunt wrote:
> > src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml, line 1210
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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/dif

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

[Patrick Hunt]

---
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)

[Patrick Hunt]

---
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


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


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



src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml


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


3.5.0



src/java/main/org/apache/zookeeper/server/ExpiryQueue.java


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


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/
> 

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

[Skye Wanderman-Milne]

---
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)

[Skye Wanderman-Milne]

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

(Updated Dec. 18, 2012, 8:14 a.m.)


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


Changes
---

Added CommandsTest, documentation to the ZooKeeper Admin's Guide, rebased on 
trunk, small fixes/improvements from comments.

CommandsTest verifies that each of the Commands outputs a Map with the expected 
keys and value types, the idea being to prevent accidental deviation from the 
specified API. It also runs each command and checks that there's no error. Does 
anyone have other ideas re: what to test?

The documentation in the Admin's Guide is a little sparse right now, but I'll 
add more once all the planned features are in.


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 0bb2317 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java eade1d6 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
  src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
  src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java cbe35fd 
  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 31f2785 
  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 7bb7b2f 
  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/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/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/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 
  src/java/test/org/apache/zookeeper/test/ClientBase.java 94f1cb0 

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)

[Patrick Hunt]

> On Dec. 14, 2012, 5:39 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/admin/AdminServer.java, lines 
> > 56-58
> > 
> >
> > Configurable properties such as this need to be in the config file, not 
> > a system property.
> 
> Skye Wanderman-Milne wrote:
> System properties can be set via the config file; see 
> QuorumPeerConfig.java:206.

You're right - honestly I've never noticed that one before!


- Patrick


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


On Nov. 30, 2012, 9:01 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 30, 2012, 9:01 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 4d09b43 
>   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 4e3a87d 
>   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/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Skye Wanderman-Milne]

> On Dec. 14, 2012, 5:39 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/admin/AdminServer.java, lines 
> > 56-58
> > 
> >
> > Configurable properties such as this need to be in the config file, not 
> > a system property.

System properties can be set via the config file; see QuorumPeerConfig.java:206.


- Skye


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


On Nov. 30, 2012, 9:01 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 30, 2012, 9:01 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 4d09b43 
>   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 4e3a87d 
>   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/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Patrick Hunt]

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


Noticed some issues with configuration and documentation. The system properties 
should be moved into the config file. Also we need to document this feature and 
the configuration.

At the very least basic unit tests need to be added.

It would be good to attach the latest version of the patch to ZOOKEEPER-1346 
and "submit" the jira so that qabot can take a pass on it.



src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java


whether or not the server is started should be made configurable. On by 
default is fine.



src/java/main/org/apache/zookeeper/server/admin/AdminServer.java


Configurable properties such as this need to be in the config file, not a 
system property.


- Patrick Hunt


On Nov. 30, 2012, 9:01 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 30, 2012, 9:01 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 4d09b43 
>   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 4e3a87d 
>   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/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Henry Robinson]

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

Ship it!


Looks good to me.


src/java/main/org/apache/zookeeper/server/DataTree.java


Long, not long



src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java


fwiw, I bet the difference is completely negligible. (I know this isn't 
your comment). No need to change it though. 



src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java


space after )



src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java


Space after )



src/java/main/org/apache/zookeeper/server/admin/Commands.java


Oh for Guava, and ImmutableMap.of(..) or something like it.


- Henry Robinson


On Nov. 30, 2012, 9:01 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 30, 2012, 9:01 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 4d09b43 
>   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 4e3a87d 
>   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/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Skye Wanderman-Milne]

> On Nov. 18, 2012, 4:10 p.m., Camille Fournier wrote:
> > src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java, line 
> > 306
> > 
> >
> > Why make this a more restrictive type?
> 
> Skye Wanderman-Milne wrote:
> If you look at ConsCommand or StatCommand in Commands.java, I clone the 
> HashSet returned by getConnections(). I copied this code from the original 
> 4lw's in NIOServerCnxn.java, including the comment that says "clone should be 
> faster than iteration ie give up the cnxns lock faster". The original code 
> clones factory.cnxns directly, but since Commands is in a different package I 
> have to use factory.getCommands(), so I changed the return type in order to 
> use clone.
> 
> I could avoid changing the return type by using a copy constructor 
> instead of clone(), but I didn't want to change the command code based on the 
> comment. FWIW, NettyServerCnxn uses a copy constructor instead of clone(), so 
> maybe it's fine.
> 
> Patrick Hunt wrote:
> You might not want to change this for b/w compat reasons, all else being 
> equal. It's not part of the "public" api, however some teams interact with 
> factories, esp in their test code, and changing this might cause problems for 
> them.
> 
> Skye Wanderman-Milne wrote:
> I'll preserve the API and use a copy ctor instead of clone, then.
> 
> Henry Robinson wrote:
> Just saw this conversation - I suggest preserving the API but doing the 
> work of the commands inside the ServerCnxnFactory. You might be able to avoid 
> the copy completely this way.

Good idea, fixed.


- Skye


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


On Nov. 30, 2012, 9:01 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 30, 2012, 9:01 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 4d09b43 
>   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 4e3a87d 
>   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/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Skye Wanderman-Milne]

> On Nov. 28, 2012, 6:19 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java, line 111
> > 
> >
> > This needs to be configurable from the get go.
> 
> Skye Wanderman-Milne wrote:
> I'll add configuration options for the base URL, port, anything else I 
> can think of.

Added system properties for port, base URL, and commands URL.


- Skye


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


On Nov. 30, 2012, 9:01 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 30, 2012, 9:01 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 4d09b43 
>   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 4e3a87d 
>   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/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Skye Wanderman-Milne]

> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java, line 110
> > 
> >
> > Can you expand the TODO to be clearer about what you want to happen? It 
> > might not be you that fixes it, such is open source.

I fixed it :)


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/WatchManager.java, line 176
> > 
> >
> > I'm not a huge fan of calling these 'dump' - that always implies 
> > writing to a string or to stderr or similar.  How about just 'getWatches' 
> > etc.?


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/WatchManager.java, line 179
> > 
> >
> > Nit: You're boxing the long every time you do a get or a put, you might 
> > as well make this a Long id instead.


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/WatchManager.java, line 182
> > 
> >
> > Nit: avoid the repeated lookup by id and make the HashSet a local 
> > variable, then do id2paths.put after the for loop.


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/admin/CommandBase.java, line 35
> > 
> >
> > Why String[]? Prefer List pretty much everywhere. You can use 
> > Arrays.asList("name1", "name2") etc. to pass in names.

No good reason, I like using arrays where I would use tuples in python :) Fixed.


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/admin/Commands.java, line 57
> > 
> >
> > You don't need this blank line


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/admin/Commands.java, line 61
> > 
> >
> > final static variables usually have capitalised names, so COMMANDS 
> > here, and PRIMARY_NAMES below.

I removed the final modifier because I didn't want to capitalize the variable 
names :) I generally use capitals to denote _immutable_ static final variables 
(or at least variables that should be treated as immutable). commands and 
primaryNames are modified when you register a Command so I think it'd be 
confusing to have them in all caps. Personally I like marking variables as 
final when possible so the compiler reminds you to initialize them, even if 
they're mutable, but it's not necessary.


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/admin/Commands.java, line 111
> > 
> >
> > Not sure we need this


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/admin/Commands.java, line 159
> > 
> >
> > New line for second }


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java, line 58
> > 
> >
> > printStackTrace is bad because it doesn't use the logging setup. Use 
> > LOG.warn("...", e) in all cases.


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java, 
> > line 128
> > 
> >
> > Remove this line (and the one above). Shouldn't this return an empty 
> > Map instead of null, in case this ever gets used?


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/admin/Commands.java, line 175
> > 
> >
> > You can't lock on factory.getConnections(), since getConnections is 
> > entitled to return different objects on different invocations (e.g. what if 
> > it copied the cnxns object and returned the copy?) 
> > 
> > My suggestion: move this logic into ServerCnxnFactory and make it a 
> > public method that returns the List>. Same with the 
> > other places you follow a similar pattern.


> On Nov. 28, 2012, 7:32 p.m., Henry Robinson wrote:
> > src/java/main/org/apache/zookeeper/server/DataTree.java, line 1184
> > 
> >
> > Good time to add some Javadoc to some of these public methods.


> O

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

[Skye Wanderman-Milne]

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

(Updated Nov. 30, 2012, 9:01 p.m.)


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


Changes
---

This patch should address all the comments EXCEPT adding a new CommandOutputter 
that outputs the original unstructured text and refactoring the existing 4lw's 
to use Commands. I'll upload a new patch with those additions soon; I'm trying 
to keep these reviews somewhat manageable.

There are a lot of changes here but most of them are small (comments, renaming, 
minor refactoring, etc.). The biggest change is how I start the AdminServer. I 
added AdminServer.setZooKeeperServer, and create the AdminServer before 
creating a ZooKeeperServer. If no server is set, commands will fail with error 
"This ZooKeeper instance is not currently serving requests". This duplicates 
the functionality of the current 4lws. An AdminServer is also started in 
QuorumPeerMain now, not just ZooKeeperServerMain.


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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
  src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
  src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
  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 31f2785 
  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 7bb7b2f 
  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/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/JsonOutputter.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/quorum/Leader.java 4d09b43 
  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 4e3a87d 
  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/server/PrepRequestProcessorTest.java 
8665bac 
  src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 

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)

[Henry Robinson]

> On Nov. 18, 2012, 4:10 p.m., Camille Fournier wrote:
> > src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java, line 
> > 306
> > 
> >
> > Why make this a more restrictive type?
> 
> Skye Wanderman-Milne wrote:
> If you look at ConsCommand or StatCommand in Commands.java, I clone the 
> HashSet returned by getConnections(). I copied this code from the original 
> 4lw's in NIOServerCnxn.java, including the comment that says "clone should be 
> faster than iteration ie give up the cnxns lock faster". The original code 
> clones factory.cnxns directly, but since Commands is in a different package I 
> have to use factory.getCommands(), so I changed the return type in order to 
> use clone.
> 
> I could avoid changing the return type by using a copy constructor 
> instead of clone(), but I didn't want to change the command code based on the 
> comment. FWIW, NettyServerCnxn uses a copy constructor instead of clone(), so 
> maybe it's fine.
> 
> Patrick Hunt wrote:
> You might not want to change this for b/w compat reasons, all else being 
> equal. It's not part of the "public" api, however some teams interact with 
> factories, esp in their test code, and changing this might cause problems for 
> them.
> 
> Skye Wanderman-Milne wrote:
> I'll preserve the API and use a copy ctor instead of clone, then.

Just saw this conversation - I suggest preserving the API but doing the work of 
the commands inside the ServerCnxnFactory. You might be able to avoid the copy 
completely this way.


- Henry


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


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Henry Robinson]

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


Looking good.


src/java/main/org/apache/zookeeper/server/DataTree.java


Good time to add some Javadoc to some of these public methods.



src/java/main/org/apache/zookeeper/server/WatchManager.java


I'm not a huge fan of calling these 'dump' - that always implies writing to 
a string or to stderr or similar.  How about just 'getWatches' etc.?



src/java/main/org/apache/zookeeper/server/WatchManager.java


Nit: You're boxing the long every time you do a get or a put, you might as 
well make this a Long id instead.



src/java/main/org/apache/zookeeper/server/WatchManager.java


Nit: avoid the repeated lookup by id and make the HashSet a local variable, 
then do id2paths.put after the for loop.



src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java


Can you expand the TODO to be clearer about what you want to happen? It 
might not be you that fixes it, such is open source.



src/java/main/org/apache/zookeeper/server/ZooTrace.java


*If* this is used from multiple threads (is it?), it should be synchronized 
on the class lock, otherwise there's no guarantee that changes to traceMask are 
visible on other threads.



src/java/main/org/apache/zookeeper/server/admin/AdminServer.java


I prefer starting servers in a separate method outside of the constructor. 
That way you can always initialise variables of type AdminServer in their usual 
place without having to wait until you know you're ok with starting the server 
before you construct one.



src/java/main/org/apache/zookeeper/server/admin/AdminServer.java


LOG.warn("Problem starting AdminServer", e);

Why don't you want to throw the exception? How can clients know if this 
worked or not?



src/java/main/org/apache/zookeeper/server/admin/CommandBase.java


Why String[]? Prefer List pretty much everywhere. You can use 
Arrays.asList("name1", "name2") etc. to pass in names.



src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java


Outputter is a bit clumsy - would Writer work better?



src/java/main/org/apache/zookeeper/server/admin/Commands.java


You don't need this blank line



src/java/main/org/apache/zookeeper/server/admin/Commands.java


final static variables usually have capitalised names, so COMMANDS here, 
and PRIMARY_NAMES below.



src/java/main/org/apache/zookeeper/server/admin/Commands.java


Not sure we need this



src/java/main/org/apache/zookeeper/server/admin/Commands.java


These guys all need a short comment to explain what they do.



src/java/main/org/apache/zookeeper/server/admin/Commands.java


New line for second }



src/java/main/org/apache/zookeeper/server/admin/Commands.java


You can't lock on factory.getConnections(), since getConnections is 
entitled to return different objects on different invocations (e.g. what if it 
copied the cnxns object and returned the copy?) 

My suggestion: move this logic into ServerCnxnFactory and make it a public 
method that returns the List>. Same with the other places 
you follow a similar pattern.



src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java


printStackTrace is bad because it doesn't use the logging setup. Use 
LOG.warn("...", e) in all cases.



src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java


Remove this line (and the one above). Shouldn't this return an empty Map 
instead of null, in case this ever gets used?


- Henry Robinson


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Hen

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

[Skye Wanderman-Milne]

> On Nov. 28, 2012, 6:19 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/DataTree.java, line 1216
> > 
> >
> > this doesn't need synchronization?

I should be synchronizing and copying the HashSet values of the ephemerals map, 
yes. I'll fix this.


> On Nov. 28, 2012, 6:19 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java, line 111
> > 
> >
> > This needs to be configurable from the get go.

I'll add configuration options for the base URL, port, anything else I can 
think of.


> On Nov. 28, 2012, 6:19 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/server/admin/AdminServer.java, line 98
> > 
> >
> > Even if we only support json initially, we should allow for other 
> > output formats to be supported eventually. Does this assumption limit us in 
> > any way going forward? i.e. is it ok to assume json output? Or should the 
> > user be somehow specifying this?

In the next code review I'm going to add support for text (i.e. the original) 
output, as well as specifying the output format.


- Skye


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


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Skye Wanderman-Milne]

> On Nov. 18, 2012, 4:10 p.m., Camille Fournier wrote:
> > src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java, line 
> > 306
> > 
> >
> > Why make this a more restrictive type?
> 
> Skye Wanderman-Milne wrote:
> If you look at ConsCommand or StatCommand in Commands.java, I clone the 
> HashSet returned by getConnections(). I copied this code from the original 
> 4lw's in NIOServerCnxn.java, including the comment that says "clone should be 
> faster than iteration ie give up the cnxns lock faster". The original code 
> clones factory.cnxns directly, but since Commands is in a different package I 
> have to use factory.getCommands(), so I changed the return type in order to 
> use clone.
> 
> I could avoid changing the return type by using a copy constructor 
> instead of clone(), but I didn't want to change the command code based on the 
> comment. FWIW, NettyServerCnxn uses a copy constructor instead of clone(), so 
> maybe it's fine.
> 
> Patrick Hunt wrote:
> You might not want to change this for b/w compat reasons, all else being 
> equal. It's not part of the "public" api, however some teams interact with 
> factories, esp in their test code, and changing this might cause problems for 
> them.

I'll preserve the API and use a copy ctor instead of clone, then.


- Skye


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


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Patrick Hunt]

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


This looks good to me overall short of the following comments.


src/java/main/org/apache/zookeeper/server/DataTree.java


this doesn't need synchronization?



src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java


This needs to be configurable from the get go.



src/java/main/org/apache/zookeeper/server/admin/AdminServer.java


The base url should be configurable.



src/java/main/org/apache/zookeeper/server/admin/AdminServer.java


Even if we only support json initially, we should allow for other output 
formats to be supported eventually. Does this assumption limit us in any way 
going forward? i.e. is it ok to assume json output? Or should the user be 
somehow specifying this?


- Patrick Hunt


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Patrick Hunt]

> On Nov. 18, 2012, 4:10 p.m., Camille Fournier wrote:
> > src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java, line 
> > 306
> > 
> >
> > Why make this a more restrictive type?
> 
> Skye Wanderman-Milne wrote:
> If you look at ConsCommand or StatCommand in Commands.java, I clone the 
> HashSet returned by getConnections(). I copied this code from the original 
> 4lw's in NIOServerCnxn.java, including the comment that says "clone should be 
> faster than iteration ie give up the cnxns lock faster". The original code 
> clones factory.cnxns directly, but since Commands is in a different package I 
> have to use factory.getCommands(), so I changed the return type in order to 
> use clone.
> 
> I could avoid changing the return type by using a copy constructor 
> instead of clone(), but I didn't want to change the command code based on the 
> comment. FWIW, NettyServerCnxn uses a copy constructor instead of clone(), so 
> maybe it's fine.

You might not want to change this for b/w compat reasons, all else being equal. 
It's not part of the "public" api, however some teams interact with factories, 
esp in their test code, and changing this might cause problems for them.


- Patrick


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


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Skye Wanderman-Milne]

> On Nov. 18, 2012, 4:21 p.m., Camille Fournier wrote:
> > src/java/main/org/apache/zookeeper/server/admin/Commands.java, line 94
> > 
> >
> > Why use a LinkedHashMap here?

No reason, I'll change it to a HashMap. (I originally displayed the commands on 
the /commands page in the order they were registered, but then I changed it to 
alphabetical order.)


- Skye


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


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Skye Wanderman-Milne]

> On Nov. 18, 2012, 4:10 p.m., Camille Fournier wrote:
> > src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java, line 
> > 306
> > 
> >
> > Why make this a more restrictive type?

If you look at ConsCommand or StatCommand in Commands.java, I clone the HashSet 
returned by getConnections(). I copied this code from the original 4lw's in 
NIOServerCnxn.java, including the comment that says "clone should be faster 
than iteration ie give up the cnxns lock faster". The original code clones 
factory.cnxns directly, but since Commands is in a different package I have to 
use factory.getCommands(), so I changed the return type in order to use clone.

I could avoid changing the return type by using a copy constructor instead of 
clone(), but I didn't want to change the command code based on the comment. 
FWIW, NettyServerCnxn uses a copy constructor instead of clone(), so maybe it's 
fine.


> On Nov. 18, 2012, 4:10 p.m., Camille Fournier wrote:
> > src/java/main/org/apache/zookeeper/server/WatchManager.java, line 199
> > 
> >
> > Ditto on dumpWatches with the PrintWriter. Let's have one place for the 
> > logic for building up what should be written, and use the built object to 
> > print so if we decide to change the logic we only have to change it once.

I totally agree. I forgot to mention it earlier, but for this initial review I 
left all the original 4lw code untouched (partly so we can focus on just the 
new code/functionality for now, and partly because reimplementing + refactoring 
all the original formatting of the 4lw's is a pain :)). I'll start on combining 
the code and will post it in the next review.


> On Nov. 18, 2012, 4:10 p.m., Camille Fournier wrote:
> > src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java, line 110
> > 
> >
> > What's with the TODO?

Just a reminder for me to start an AdminServer for each server in a quorum. 
Right now it only starts in standalone mode.


- Skye


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


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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
> 

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

[Camille Fournier]

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


Those are just some initial comments (on a spotty connection on a train). Looks 
like a great start though.

- Camille Fournier


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Camille Fournier]

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



src/java/main/org/apache/zookeeper/server/admin/Commands.java


Why use a LinkedHashMap here?


- Camille Fournier


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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)

[Camille Fournier]

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



src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java


Why make this a more restrictive type?



src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java


Maybe you should rewrite the dumpSessions that uses the PrintWriter to 
print the output of this method instead of duplicating some of the logic



src/java/main/org/apache/zookeeper/server/WatchManager.java


Ditto on dumpWatches with the PrintWriter. Let's have one place for the 
logic for building up what should be written, and use the built object to print 
so if we decide to change the logic we only have to change it once.



src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java


What's with the TODO?


- Camille Fournier


On Nov. 16, 2012, 11:25 p.m., Skye Wanderman-Milne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> ---
> 
> (Updated Nov. 16, 2012, 11:25 p.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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   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 31f2785 
>   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 7bb7b2f 
>   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/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/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> 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
> 
>

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

[Skye Wanderman-Milne]

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

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/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java eec2f2a 
  src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
  src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
  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 31f2785 
  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 7bb7b2f 
  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/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/JsonOutputter.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
3182419 
  src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
8665bac 
  src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 

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