Return-Path: X-Original-To: apmail-river-dev-archive@www.apache.org Delivered-To: apmail-river-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 4A53E101E5 for ; Mon, 3 Feb 2014 10:48:13 +0000 (UTC) Received: (qmail 26833 invoked by uid 500); 3 Feb 2014 10:48:13 -0000 Delivered-To: apmail-river-dev-archive@river.apache.org Received: (qmail 26470 invoked by uid 500); 3 Feb 2014 10:48:05 -0000 Mailing-List: contact dev-help@river.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@river.apache.org Delivered-To: mailing list dev@river.apache.org Received: (qmail 26458 invoked by uid 99); 3 Feb 2014 10:48:02 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 03 Feb 2014 10:48:02 +0000 X-ASF-Spam-Status: No, hits=-0.0 required=5.0 tests=SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: local policy) Received: from [207.57.65.70] (HELO zeus.net.au) (207.57.65.70) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 03 Feb 2014 10:47:55 +0000 Received: (qmail 86631 invoked by uid 16710); 3 Feb 2014 10:47:31 -0000 Received: from unknown (HELO [10.50.76.172]) (peter.firmstone@[49.181.201.50]) (envelope-sender ) by 207.57.65.70 (qmail-ldap-1.03) with AES256-SHA encrypted SMTP for ; 3 Feb 2014 10:47:31 -0000 Message-ID: <52EF73BD.4090906@zeus.net.au> Date: Mon, 03 Feb 2014 20:47:25 +1000 From: Peter Firmstone User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:1.9.2.28) Gecko/20120306 Thunderbird/3.1.20 MIME-Version: 1.0 To: dev@river.apache.org Subject: Discussion of recent changes to ServiceDiscoveryManager and Reggie. Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Checked: Checked by ClamAV on apache.org Replacement of TaskManager with ExecutorService. ServiceDiscoveryManager: 1.Complex dependency relationships. 2.Dependency is the exception, not the norm, existing runAfter(List task, int position) implementations, changed to dependsOn(CacheTask t), iteration being performed by the caller rather than the task implementation. 3.Used a wrapper ExecutorService to customise FutureTask to implement Observer call backs to notify dependant waiting tasks. 4.In addition to call backs, implemented subscriber to allow tasks to fire off new tasks and notify their dependants to wait for these also. 5.Dependant tasks manage a small lists of tasks they�re waiting to be notified for completion, when the last precedent task is removed, the dependant task is submitted to the executor. Fixed a number of race conditions that were revealed as a result of replacing TaskManager. TaskManager masks data races by making code that submit and polls tasks relatively single threaded. The multiple threads in TaskManager were useful to hand off network calls. Reggie: 1.Many dependencies, with a simple relationship, to quote Patricia: EventTask - run after EventTask for same listener, "Keep events going to the same listener ordered"; 2.Not suited to the call back implementation in SDM. 3.Better suited to a simple priority queue, for tasks that were dependant to be executed in order, but only dependant tasks, those tasks with the same listener. 4.Dependant tasks must be single threaded. 5.Rather than have many single threaded executors, I used a number of priority queues, one for each listener, that utilise call backs to remove a task after it completes, preventing execution of dependant tasks. 6.Although dependency relationships have been preserved, tests expect remote event listeners to be notified in the order they register with the ServiceRegistrar, this behaves as expected with TaskManager, but ExecutorService is asynchronous and listeners are notified out of order. Yes that's right, to be compliant with qa tests, not only must events for each listener be ordered, but the listeners themselves must be notified in the order or their registration with the lookup service. 7.In the new implementation, Events are still ordered correctly to each listener, it�s just the first listener that registered may not be notified first when an event occurs because of the asynchronous nature of ExecutorService implementations, unlike TaskManager. 8.We need to consider the impact of maintaining 100% backward compatible behaviour on performance and complexity. This can be managed by ensuring there is a bottleneck in the ExecutorService, by limiting performance, order of notification to listeners is preserved. A fixed length queue with an ExecutorService that only creates new threads once the queue is full should do the trick. a.This may sound brittle, but it�s the same guarantee that TaskManager provided, so the default ExecutorService can be performance limited just like TaskManager. b.For those who want to tune for all out performance and don�t care which listener is notified first, they can configure their own ExecutorService implementation. c.The complex option of creating dependency links between listeners harms performance, so there�s no point introducing this type of dependency as it forces everyone�s performance to suffer on the altar of compatibility. What are your thoughts? Peter.