+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/
