> On Nov. 18, 2012, 4:10 p.m., Camille Fournier wrote: > > src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java, line > > 306 > > <https://reviews.apache.org/r/8094/diff/1/?file=190875#file190875line306> > > > > 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 > >
