Return-Path: X-Original-To: apmail-stratos-dev-archive@minotaur.apache.org Delivered-To: apmail-stratos-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 72E7510E26 for ; Wed, 11 Dec 2013 08:29:42 +0000 (UTC) Received: (qmail 72586 invoked by uid 500); 11 Dec 2013 08:29:37 -0000 Delivered-To: apmail-stratos-dev-archive@stratos.apache.org Received: (qmail 71955 invoked by uid 500); 11 Dec 2013 08:29:22 -0000 Mailing-List: contact dev-help@stratos.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@stratos.incubator.apache.org Delivered-To: mailing list dev@stratos.incubator.apache.org Received: (qmail 71942 invoked by uid 99); 11 Dec 2013 08:29:20 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 11 Dec 2013 08:29:20 +0000 X-ASF-Spam-Status: No, hits=1.7 required=5.0 tests=FREEMAIL_ENVFROM_END_DIGIT,HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of chsnow123@gmail.com designates 209.85.160.44 as permitted sender) Received: from [209.85.160.44] (HELO mail-pb0-f44.google.com) (209.85.160.44) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 11 Dec 2013 08:29:14 +0000 Received: by mail-pb0-f44.google.com with SMTP id rq2so9449724pbb.17 for ; Wed, 11 Dec 2013 00:28:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=QyeVggh8nd4AL6lBJR9xHdV5uTb/kx+AWUHNMYSHLxw=; b=tL7Wh7N8s/dx3PnbQoqUrQrmxnvGCU4p7fXtCGBPDZJY5Nx4Cr7cG33ABy2XqQe4dF mZSDtpaz62PZCebVADZKW6uju2tTuTnDMbBgk97JD9heTUnTFYba+OpXVHjUyUJfn2Bs 2YZdKlsUYX70t+9QqtiIdq6oQA5IBeKWLEj2nvH9znHsbAjjHz2W7fSLRMpiJTGi7/U2 1QTdwCidlITiHqS9e7mFpC0BSFCpIsCBHf/27IBB0prIMc1chzJTb9Y3VwAGEDp9T1IH tVSRx5PEEY5u9lF/To9SqChOvqZa0MgfVGgZ5a1R00kD0ncWgnLVcYcdahvwDXZlk1Vg VvTQ== MIME-Version: 1.0 X-Received: by 10.66.14.9 with SMTP id l9mr167827pac.57.1386750533002; Wed, 11 Dec 2013 00:28:53 -0800 (PST) Received: by 10.68.16.194 with HTTP; Wed, 11 Dec 2013 00:28:52 -0800 (PST) In-Reply-To: References: Date: Wed, 11 Dec 2013 08:28:52 +0000 Message-ID: Subject: Re: broken build From: chris snow To: dev@stratos.incubator.apache.org Content-Type: multipart/alternative; boundary=bcaec520e955e96b2a04ed3e041a X-Virus-Checked: Checked by ClamAV on apache.org --bcaec520e955e96b2a04ed3e041a Content-Type: text/plain; charset=ISO-8859-1 Hi Imesh, While looking through the LoadBalancerInFlightRequestCountNotifier I was wondering if there may be a situation where the LoadBalancerInFlightRequestCountNotifier may never acquire the lock because the ReenterantReadWriteLock is using the default non-Fair policy: "A nonfair lock that is continously contended may indefinitely postpone one or more reader or writer threads, but will normally have higher throughput than a fair lock." One option to prevent indefinite waiting would be to change the lock acquisition order in TopologyManager to be 'Fair' by passing the fair flag in the constructor: private static volatile ReentrantReadWriteLock lock = new ReentrantReadWriteLock(true); What do you think - would this change be useful in TopologyManager and TenantManager? Many thanks, Chris On Wed, Dec 11, 2013 at 5:54 AM, Imesh Gunaratne wrote: > Thanks Chris! I'm glad it helped. > BTW it's really good that many people look into implementation details of > each product, so that we could find space for improvements very easily. > > > > On Wed, Dec 11, 2013 at 10:54 AM, chris snow wrote: > >> No worries Imesh. Although I didn't have enough knowledge of the code >> base to work out how to fix it, investigating the problem was a really >> useful learning exercise! >> On 11 Dec 2013 04:51, "Imesh Gunaratne" wrote: >> >>> I'm sorry for the inconvenience caused. I have now fixed this issue and >>> checked in the modification to remote git. >>> >>> if (statsPublisher.isEnabled()) { >>> Collection partitionIds; >>> for (Service service : >>> TopologyManager.getTopology().getServices()) { >>> for (Cluster cluster : service.getClusters()) { >>> partitionIds = cluster.findPartitionIds(); >>> for(String partitionId : partitionIds) { >>> >>> statsPublisher.publish(cluster.getClusterId(), partitionId, >>> statsReader.getInFlightRequestCount(cluster.getClusterId(), partitionId)); >>> } >>> } >>> } >>> } >>> >>> >>> Thanks >>> Imesh >>> >>> >>> On Wed, Dec 11, 2013 at 2:57 AM, chris snow wrote: >>> >>>> There was a recent change to the LoadBalancerStatsReader that is >>>> resulting in a build error: >>>> >>>> - int getInFlightRequestCount(String clusterId); >>>> + int getInFlightRequestCount(String clusterId, String partitionId); >>>> >>>> >>>> This is the build error: >>>> >>>> >>>> Waiting for Jenkins to finish collecting data[ERROR] Failed to execute >>>> goal org.apache.maven.plugins:maven-compiler-plugin:2.5.1:compile >>>> (default-compile) on project >>>> org.apache.stratos.load.balancer.extension.api: Compilation failure [ERROR] >>>> /var/lib/jenkins/jobs/stratos/workspace/components/org.apache.stratos.load.balancer.extension.api/src/main/java/org/apache/stratos/load/balancer/extension/api/LoadBalancerInFlightRequestCountNotifier.java:[62,86] >>>> error: method getInFlightRequestCount in interface LoadBalancerStatsReader >>>> cannot be applied to given types; [ERROR] -> [Help 1] [ERROR] >>>> >>>> I've put a temporary fix into LoadBalancerInFlightRequestCountNotifier >>>> by providing an empty partitionId string value: >>>> >>>> @Override >>>> public void run() { >>>> while (!terminated) { >>>> try { >>>> try { >>>> Thread.sleep(statsPublisherInterval); >>>> } catch (InterruptedException ignore) { >>>> } >>>> >>>> if (statsPublisher.isEnabled()) { >>>> for (Service service : >>>> TopologyManager.getTopology().getServices()) { >>>> for (Cluster cluster : service.getClusters()) { >>>> statsPublisher.publish( >>>> cluster.getClusterId(), >>>> "", /* FIXME: how to get the partitionId? >>>> */ >>>> statsReader.getInFlightRequestCount(cluster.getClusterId(), >>>> "") /* FIXME: how to get the partitionId? */ >>>> ); >>>> } >>>> } >>>> } else if (log.isWarnEnabled()) { >>>> log.warn("CEP statistics publisher is disabled"); >>>> } >>>> } catch (Exception e) { >>>> if (log.isErrorEnabled()) { >>>> log.error("Could not publish in-flight request >>>> count", e); >>>> } >>>> } >>>> } >>>> } >>>> >>>> I'll do some more investigation ... >>>> >>> >>> > -- Check out my professional profile and connect with me on LinkedIn. http://lnkd.in/cw5k69 --bcaec520e955e96b2a04ed3e041a Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
Hi Imesh,

While looking through the LoadBalancerInF= lightRequestCountNotifier I was wondering if there may be a situation where= the LoadBalancerInFlightRequestCountNotifier may never acquire the lock be= cause the ReenterantReadWriteLock is using the default non-Fair policy:

=A0 =A0"A nonfair lock that is continously contended may indefinit= ely postpone one or more reader or writer threads, but will normally have h= igher throughput than a fair lock."

One option to prevent indef= inite waiting would be to change the lock acquisition order in TopologyMana= ger to be 'Fair' by passing the fair flag in the constructor:

private static volatile ReentrantReadWriteLock lock =3D new ReentrantRe= adWriteLock(true);

What do you think - would this change be useful i= n TopologyManager and TenantManager?

Many thanks,

Chris


On Wed,= Dec 11, 2013 at 5:54 AM, Imesh Gunaratne <imesh@apache.org> = wrote:
Thanks Chris! I'm glad = it helped.
BTW it's really good that many people look into implemen= tation details of each product, so that we could find space for improvement= s very easily.


On Wed, Dec 11, 2013 at 10:54 AM, chris sn= ow <chsnow123@gmail.com> wrote:

No worries Imesh.=A0 Although= I didn't have enough knowledge of the code base to work out how to fix= it, investigating the problem was a really useful learning exercise!

On 11 Dec 2013 04:51, "Imesh Gunaratne"= ; <imesh@apache.or= g> wrote:
I'm sorry for the inconvenience caused. I have no= w fixed this issue and checked in the modification to remote git.
=

if (statsPublisher.isEnabled()) {
=A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Collection<String> partitionIds;<= /div>
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (Service service : Topolog= yManager.getTopology().getServices()) {
=A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 for (Cluster cluster : service.getClusters()) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 partitionIds =3D =A0cluster.findPart= itionIds();
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 for(String par= titionId : partitionIds) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 statsPublisher.publish(cluster.getClusterId(), = partitionId, statsReader.getInFlightRequestCount(cluster.getClusterId(), pa= rtitionId));
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
=A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 }
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }


Thanks
Imesh


On Wed, Dec 11, 2013 at 2:57 AM, chris s= now <chsnow123@gmail.com> wrote:
There was a recent change to the=A0LoadBa= lancerStatsReader that is resulting in a build error:

- =A0 =A0int getInFlightReques= tCount(String clusterId);
+ =A0 =A0int getInFlightRequestCount(String clusterId, Str= ing partitionId);

This is the = build error:
Waiting for Jenkins to finish collecting data[ERROR] Failed to= execute goal org.apache.maven.plugins:maven-compiler-plugin:2.5.1:compile = (default-compile) on project org.apache.stratos.load.balancer.extension.api= : Compilation failure [ERROR] /var/lib/jenkins/jobs/stratos/workspace/compo= nents/org.apache.stratos.load.balancer.extension.api/src/main/java/org/apac= he/stratos/load/balancer/extension/api/LoadBalancerInFlightRequestCountNoti= fier.java:[62,86] error: method getInFlightRequestCount in interface LoadBa= lancerStatsReader cannot be applied to given types; [ERROR] -> [Help 1] = [ERROR]

I've put a temporary fix into LoadBalan= cerInFlightRequestCountNotifier by providing an empty partitionId string va= lue:

=A0 = =A0@Override
=A0 =A0 public void run() {
=A0 =A0 = =A0 =A0 while (!terminated) {
=A0 =A0 =A0 =A0 =A0 =A0= try {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 try {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Thread.sleep(statsPub= lisherInterval);
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } catch (InterruptedExcept= ion ignore) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
=

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= if (statsPublisher.isEnabled()) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (Service service : TopologyMana= ger.getTopology().getServices()) {
=A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 for (Cluster cluster : service.getClusters(= )) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 statsPublisher.publish(
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cluster.getClusterId(),=A0
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 <= span style=3D"white-space:pre-wrap"> "", /* FIXME: how to= get the partitionId? */
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 statsReader.getInFlightRequestC= ount(cluster.getClusterId(), "") /* FIXME: how to get the partiti= onId? */
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 );
=A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 }
=A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 }
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } e= lse if (log.isWarnEnabled()) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 log.warn("CEP= statistics publisher is disabled");
=A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 }
=A0 =A0 =A0 =A0 =A0 =A0 } catc= h (Exception e) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if = (log.isErrorEnabled()) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 log.error("Co= uld not publish in-flight request count", e);
= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
=A0 =A0 =A0 =A0 =A0= =A0 }
=A0 =A0 =A0 =A0 }
=A0 =A0 }

I'll do some more invest= igation ...





--
=
Check out my professional profile and connect with me= on LinkedIn. http://ln= kd.in/cw5k69
--bcaec520e955e96b2a04ed3e041a--