> On July 8, 2014, 3:44 p.m., Raul Gutierrez Segales wrote: > > src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java, line > > 417 > > <https://reviews.apache.org/r/23259/diff/1/?file=623498#file623498line417> > > > > why not clone cnxns here as well? a couple of lines below it says that > > it's cheaper than this synchronized blocked.
My guess is that resetStats() (in ServerCnxn) is fast enough - just setting a bunch of fields - that it's not worth the clone, vs. getConnectionInfo() which assembles a map and calls a bunch of methods. Also, compare with NettyServerCnxn.CnxnStatResetCommand.commandRun(), which also just syncs and doesn't clone. If you still would prefer the clone, just say so and I'm happy to make the change. > On July 8, 2014, 3:44 p.m., Raul Gutierrez Segales wrote: > > src/java/main/org/apache/zookeeper/server/WatchManager.java, line 264 > > <https://reviews.apache.org/r/23259/diff/1/?file=623504#file623504line264> > > > > Hmm, passing Map<String, Object>s around can get messy in the long run. > > Why not promote them to classes (i.e.: WatchesSummary, Conf, etc.) and have > > a toMap() member (or whatever representation) there? > > > > I fear that establishing Map<>s as API might end up being too > > inflexible in the future. > > > > Thoughts? I like it. I'll start working on that. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23259/#review47463 ----------------------------------------------------------- On July 3, 2014, 9:26 a.m., Bill Havanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23259/ > ----------------------------------------------------------- > > (Updated July 3, 2014, 9:26 a.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/ZKDatabase.java 91d6bce > src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java bee35c0 > 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 2dac4ec > 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/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 > >
