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

Switch to Plain View
Zookeeper, mail # dev - Re: svn commit: r1225200 - in /zookeeper/trunk: ./ src/java/main/org/apache/zookeeper/server/ src/java/test/org/apache/zookeeper/server/quorum/ src/java/test/org/apache/zookeeper/test/
Patrick Hunt
2011-12-28, 17:45
Camille Fournier
2011-12-28, 18:12
Camille Fournier
2011-12-28, 18:31
Patrick Hunt
2011-12-28, 19:24
Ted Yu
2011-12-28, 20:58
Patrick Hunt
2011-12-28, 21:04
Camille Fournier
2011-12-28, 21:10
Patrick Hunt
2011-12-28, 21:24
Camille Fournier
2011-12-28, 21:26
Patrick Hunt
2011-12-28, 21:32
Camille Fournier
2011-12-28, 21:53
Patrick Hunt
2011-12-28, 22:12
Camille Fournier
2011-12-28, 22:21
Patrick Hunt
2011-12-28, 22:45
+
Patrick Hunt 2011-12-28, 17:45
+
Camille Fournier 2011-12-28, 18:12
+
Camille Fournier 2011-12-28, 18:31
Copy link to this message
-
Re: svn commit: r1225200 - in /zookeeper/trunk: ./ src/java/main/org/apache/zookeeper/server/ src/java/test/org/apache/zookeeper/server/quorum/ src/java/test/org/apache/zookeeper/test/
Camille Fournier 2011-12-28, 19:18
Also, Pat, the JIRA is still open. I will look to revert in a minute
although I think the sync changes may necessitate a new JIRA entirely.

On Wed, Dec 28, 2011 at 1:31 PM, Camille Fournier <[EMAIL PROTECTED]> wrote:
> After looking for a few mins, here are my observations:
>
> The implementation of HashSet in jdk 1.6 uses an underlying HashMap, which
> uses a bog-standard int for the size. So, I completely agree that we can
> get invalid results for a point in time. But does anyone really care about
> the *exact* moment-in-time number of cnxns in the system, in a system where
> cnxns are coming and going? At best you'll see a value that will likely be
> out of date the moment you read it.
>
> But, if we're going to do this, I do think there may be a case for
> refactoring to a concurrent data structure given that we are randomly
> syncing on the cnxns set in a way that makes adding additional interactions
> with it error prone in this way. And we should definitely remove that
> getConnections method, if anyone ever iterated over that set they would be
> in a sad concurrency situation.
> Seems possibly worth a whole new ticket to do this. What do you think?
>
> Thanks,
> C
>
> On Wed, Dec 28, 2011 at 1:12 PM, Camille Fournier <[EMAIL PROTECTED]>wrote:
>
>> I'm not sure it's meaningful enough to be worth the sync overhead. We
>> should look into that. The alternative if we must is using a proper
>> concurrent collection. Neha, any thoughts?
>>
>> C
>> On Dec 28, 2011 12:45 PM, "Patrick Hunt" <[EMAIL PROTECTED]> wrote:
>>
>>> I believe there is a bug in this commit. The "cnxns" size() call is
>>> not being synchronized. This will lead to invalid results at best, at
>>> worst outright failure (hard to say w/o knowing the implementation of
>>> HashSet).
>>>
>>> Camille can you work with Neha to get this fixed? Perhaps in the
>>> meantime (if it's going to take a while) you can revert this change,
>>> re-open the jira, update the patch, and reapply at some later time?
>>>
>>> Patrick
>>>
>>> On Wed, Dec 28, 2011 at 6:55 AM,  <[EMAIL PROTECTED]> wrote:
>>> > Author: camille
>>> > Date: Wed Dec 28 14:55:37 2011
>>> > New Revision: 1225200
>>> >
>>> > URL: http://svn.apache.org/viewvc?rev=1225200&view=rev
>>> > Log:
>>> > ZOOKEEPER-1321: Add number of client connections metric in JMX and srvr
>>> (Neha Narkhede via camille)
>>> >
>>> > Modified:
>>> >    zookeeper/trunk/CHANGES.txt
>>> >
>>>  zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java
>>> >
>>>  zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
>>> >
>>>  zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java
>>> >
>>>  zookeeper/trunk/src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
>>> >
>>>  zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
>>> >
>>>  zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerStats.java
>>> >
>>>  zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
>>> >
>>>  zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java
>>> >
>>>  zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ZooKeeperServerMXBean.java
>>> >
>>>  zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
>>> >
>>>  zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java
>>> >
>>> > Modified: zookeeper/trunk/CHANGES.txt
>>> > URL:
>>> http://svn.apache.org/viewvc/zookeeper/trunk/CHANGES.txt?rev=1225200&r1=1225199&r2=1225200&view=diff
>>> >
>>> =============================================================================>>> > --- zookeeper/trunk/CHANGES.txt (original)
>>> > +++ zookeeper/trunk/CHANGES.txt Wed Dec 28 14:55:37 2011
>>> > @@ -162,6 +162,8 @@ IMPROVEMENTS:
>>> >
>>> >   ZOOKEEPER-1342. quorum Listener & LearnerCnxAcceptor are missing
>>> >   thread names (Rakesh R via phunt)
>>> > +
>>> > +  ZOOKEEPER-1321. Add number of client connections metric in JMX and
+
Patrick Hunt 2011-12-28, 19:24
+
Ted Yu 2011-12-28, 20:58
+
Patrick Hunt 2011-12-28, 21:04
+
Camille Fournier 2011-12-28, 21:10
+
Patrick Hunt 2011-12-28, 21:24
+
Camille Fournier 2011-12-28, 21:26
+
Patrick Hunt 2011-12-28, 21:32
+
Camille Fournier 2011-12-28, 21:53
+
Patrick Hunt 2011-12-28, 22:12
+
Camille Fournier 2011-12-28, 22:21
+
Patrick Hunt 2011-12-28, 22:45