Thanks - I'll take a look this evening (GMT). This small change will be a good way to test my git commit access.
On Wed, Dec 11, 2013 at 9:03 AM, Nirmal Fernando <[email protected]>wrote: > +1 for making it 'fair' Chris. Otherwise some threads would starve for > long time. > > > On Wed, Dec 11, 2013 at 1:58 PM, chris snow <[email protected]> wrote: > >> 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 <[email protected]>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 <[email protected]>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" <[email protected]> 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<String> 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 <[email protected]>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 >> > > > > -- > Best Regards, > Nirmal > > Nirmal Fernando. > PPMC Member & Committer of Apache Stratos, > Senior Software Engineer, WSO2 Inc. > > Blog: http://nirmalfdo.blogspot.com/ > -- Check out my professional profile and connect with me on LinkedIn. http://lnkd.in/cw5k69
