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
+
Camille Fournier 2011-12-28, 19:18
+
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
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, 21:53
Fair enough. Syncs for all! It's a drop in the bucket compared to the fact
that we aren't handling the fundamental socket connections for 4lws
correctly anyway.

On Wed, Dec 28, 2011 at 4:32 PM, Patrick Hunt <[EMAIL PROTECTED]> wrote:

> On Wed, Dec 28, 2011 at 1:26 PM, Camille Fournier <[EMAIL PROTECTED]>
> wrote:
> > I'm not sure I agree in the assumption that monitoring pulls happen
> > infrequently...
>
> Well that would be bad news then - see "stat" command processing and
> similar. All the more reason to look at this in more depth.
>
> Patrick
>
>
> > On Dec 28, 2011 4:24 PM, "Patrick Hunt" <[EMAIL PROTECTED]> wrote:
> >
> >> They seem like distinct changes to me. In particular getting the size
> >> is going to happen infrequently (monitoring pull) so I don't see a
> >> problem fixing the existing patch in the same way the code currently
> >> handles cnxns access, with a separate jira to do the refactoring. Am I
> >> missing something?
> >>
> >> Patrick
> >>
> >> On Wed, Dec 28, 2011 at 1:10 PM, Camille Fournier <[EMAIL PROTECTED]>
> >> wrote:
> >> > I don't think creating it as a flat sync is a good idea, so if we want
> >> this
> >> > I think we need to refactor that structure to be concurrent.
> >> >
> >> > C
> >> > On Dec 28, 2011 2:24 PM, "Patrick Hunt" <[EMAIL PROTECTED]> wrote:
> >> >
> >> >> I think it needs to be fixed. It's obviously incorrect. Also if
> >> >> someone changes the underlying implementation at some point it might
> >> >> bite them.
> >> >>
> >> >> Given it wasn't applied to 3.4 branch yet I'd suggest revert, fix the
> >> >> patch, then reapply to both branches. (that's easiest/bulletproof
> imo)
> >> >>
> >> >> I'd recommend a new ticket to fix the issues you identified
> (refactor,
> >> >> etc...) Neha was looking for further items to work on, this would be
> a
> >> >> good one.
> >> >>
> >> >> Patrick
> >> >>
> >> >> On Wed, Dec 28, 2011 at 11:18 AM, Camille Fournier <
> [EMAIL PROTECTED]>
> >> >> wrote:
> >> >> > 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()
+
Patrick Hunt 2011-12-28, 22:12
+
Camille Fournier 2011-12-28, 22:21
+
Patrick Hunt 2011-12-28, 22:45