Home | About | Sematext search-lucene.com search-hadoop.com
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB
 Search Hadoop and all its subprojects:

Switch to Threaded 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/


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/
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()
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB