----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23259/#review47662 -----------------------------------------------------------
src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml <https://reviews.apache.org/r/23259/#comment83773> nit: Patrick mentioned that it would be nice to have this for 3.5.0 actually, so we might need to update this before merging (probably the committer can fix it during the merge). src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java <https://reviews.apache.org/r/23259/#comment83774> why is this different than NIOServerCnxnFactory impl if both use the same cnxns protected method (which is a ConcurrentHashSet)? On a broader note, it looks like the NettyServerCnxnFactory was never updated to reflect that ServerCnxnFactory now has cnxns as a ConcurrentHashSet. src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java <https://reviews.apache.org/r/23259/#comment83775> ditto wrt to divergence w/ NIOServerCnxnFactory's impl. src/java/main/org/apache/zookeeper/server/WatchesReport.java <https://reviews.apache.org/r/23259/#comment83776> nit: newline src/java/main/org/apache/zookeeper/server/ZooKeeperServerConf.java <https://reviews.apache.org/r/23259/#comment83777> nit: newline. src/java/main/org/apache/zookeeper/server/ZooKeeperServerConf.java <https://reviews.apache.org/r/23259/#comment83778> ditto src/java/main/org/apache/zookeeper/server/ZooKeeperServerConf.java <https://reviews.apache.org/r/23259/#comment83779> ditto src/java/main/org/apache/zookeeper/server/ZooKeeperServerConf.java <https://reviews.apache.org/r/23259/#comment83780> ditto src/java/main/org/apache/zookeeper/server/ZooKeeperServerConf.java <https://reviews.apache.org/r/23259/#comment83781> ditto src/java/main/org/apache/zookeeper/server/ZooKeeperServerConf.java <https://reviews.apache.org/r/23259/#comment83782> ditto src/java/main/org/apache/zookeeper/server/ZooKeeperServerConf.java <https://reviews.apache.org/r/23259/#comment83783> ditto src/java/main/org/apache/zookeeper/server/admin/CommandBase.java <https://reviews.apache.org/r/23259/#comment83785> either address or remove TODO src/java/main/org/apache/zookeeper/server/admin/Commands.java <https://reviews.apache.org/r/23259/#comment83787> nit, but cheaper: Command prev = commands.put(name, command); if (prev != null) { LOG.warn("Re-registering command... "); } src/java/main/org/apache/zookeeper/server/admin/Commands.java <https://reviews.apache.org/r/23259/#comment83789> how about we use a CommandResult object here instead of a Map<>? Just because we have a nice compiler that can help with type safety :-) src/java/main/org/apache/zookeeper/server/admin/Commands.java <https://reviews.apache.org/r/23259/#comment83788> i'd say address or drop TODO src/java/main/org/apache/zookeeper/server/admin/Commands.java <https://reviews.apache.org/r/23259/#comment83790> ditto, return a CommandResult object perhaps? src/java/main/org/apache/zookeeper/server/admin/Commands.java <https://reviews.apache.org/r/23259/#comment83793> nit: trailing white space - Raul Gutierrez Segales On July 11, 2014, 3:36 p.m., Bill Havanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23259/ > ----------------------------------------------------------- > > (Updated July 11, 2014, 3:36 p.m.) > > > Review request for zookeeper. > > > Bugs: ZOOKEEPER-1346 > https://issues.apache.org/jira/browse/ZOOKEEPER-1346 > > > Repository: zookeeper-git > > > Description > ------- > > Addition of new AdminServer, with implementation using Jetty, for RESTful > handling of commands. > > Work done by Skye Wanderman-Milne; this patch is a rebase on trunk since the > last patch was provided early in 2013. > > Suggested ordering for review: > > 1. ServerCnxn, ServerCnxnFactory and implementations. > 2. DataTree, WatchManager, ZKDatabase. > 3. ExpiryQueue, SessionTracker and Impl, LeaderSessionTracker, > LearnerSessionTracker. > 4. ZooKeeperServer. > 5. org.apache.zookeeper.server.admin package > 6. Everything else (integration with admin server). > > > Diffs > ----- > > ivy.xml ad14b80 > src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml bb16fa5 > src/java/main/org/apache/zookeeper/server/DataTree.java f2a4f86 > src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e > src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 6e057a5 > src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java > 6b8d2a6 > src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 > src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java b0f8130 > src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f > src/java/main/org/apache/zookeeper/server/SessionTracker.java 4875ead > src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java c971b89 > src/java/main/org/apache/zookeeper/server/WatchManager.java 39bf82e > src/java/main/org/apache/zookeeper/server/WatchesPathReport.java > PRE-CREATION > src/java/main/org/apache/zookeeper/server/WatchesReport.java PRE-CREATION > src/java/main/org/apache/zookeeper/server/WatchesSummary.java PRE-CREATION > src/java/main/org/apache/zookeeper/server/ZKDatabase.java 91d6bce > src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java bee35c0 > src/java/main/org/apache/zookeeper/server/ZooKeeperServerConf.java > PRE-CREATION > src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 7bffaf0 > src/java/main/org/apache/zookeeper/server/ZooTrace.java 42417f5 > 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 3baf41a > src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java > e746892 > src/java/main/org/apache/zookeeper/server/quorum/Learner.java 43e1b8b > src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java > cf0ecfb > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 4ec1f9a > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java > b374c29 > > src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java > 0369eb9 > src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f > src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java > b1b44fb > src/java/test/org/apache/zookeeper/server/WatchesPathReportTest.java > PRE-CREATION > src/java/test/org/apache/zookeeper/server/WatchesReportTest.java > PRE-CREATION > src/java/test/org/apache/zookeeper/server/WatchesSummaryTest.java > PRE-CREATION > src/java/test/org/apache/zookeeper/server/ZooKeeperServerConfTest.java > PRE-CREATION > 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 > b04d2bd > src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java > f35e1cd > src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 0ea7f19 > > Diff: https://reviews.apache.org/r/23259/diff/ > > > Testing > ------- > > Compiles, unit tests pass. Tested commands "by hand" on running instance. > > > Thanks, > > Bill Havanki > >
