Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Plain View
Zookeeper, mail # dev - Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)


+
Skye Wanderman-Milne 2012-11-30, 21:14
+
Skye Wanderman-Milne 2012-11-30, 21:13
+
Skye Wanderman-Milne 2012-11-16, 23:25
+
Skye Wanderman-Milne 2012-11-30, 21:01
+
Henry Robinson 2012-12-11, 01:25
+
Patrick Hunt 2012-12-14, 17:39
+
Patrick Hunt 2012-12-14, 22:39
+
Skye Wanderman-Milne 2012-12-14, 22:28
+
Skye Wanderman-Milne 2012-12-18, 08:15
+
Skye Wanderman-Milne 2013-01-04, 02:17
+
Camille Fournier 2012-11-18, 16:24
+
Camille Fournier 2012-11-18, 16:21
+
Skye Wanderman-Milne 2012-11-19, 23:55
+
Camille Fournier 2012-11-18, 16:10
Copy link to this message
-
Re: Review Request: ZOOKEEPER-1346: Handle 4lws and monitoring on separate port (creating jetty server)
Skye Wanderman-Milne 2012-11-30, 21:17


> 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.
>
> 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
+
Skye Wanderman-Milne 2012-11-19, 23:55
+
Skye Wanderman-Milne 2013-01-15, 23:53