CMFActivity: always process the queue with higher priority instead of round robin

product/CMFActivity/Activity/Queue.py

@@ -81,8 +81,6 @@ class Queue:
   #scriptable_method_id_list = ['appendMessage', 'nextMessage', 'delMessage']
 
   def __init__(self):
-    self.is_alive = {}
-    self.is_awake = {}
     self.is_initialized = 0
 
   def initialize(self, activity_tool):
@@ -105,24 +103,9 @@ class Queue:
   def dequeueMessage(self, activity_tool, processing_node):
     pass
 
-  def tic(self, activity_tool, processing_node):
-    # Tic should return quickly to prevent locks or commit transactions at some point
-    if self.dequeueMessage(activity_tool, processing_node):
-      self.sleep(activity_tool, processing_node)
-
   def distribute(self, activity_tool, node_count):
     pass
 
-  def sleep(self, activity_tool, processing_node):
-    self.is_awake[processing_node] = 0
-
-  def wakeup(self, activity_tool, processing_node):
-    self.is_awake[processing_node] = 1
-
-  def terminate(self, activity_tool, processing_node):
-    self.is_awake[processing_node] = 0
-    self.is_alive[processing_node] = 0
-
   def validate(self, activity_tool, message, check_order_validation=1, **kw):
     """
       This is the place where activity semantics is implemented
@@ -220,9 +203,6 @@ class Queue:
     else:
       pass
 
-  def isAwake(self, activity_tool, processing_node):
-    return self.is_awake[processing_node]
-
   def hasActivity(self, activity_tool, object, processing_node=None, active_process=None, **kw):
     return 0
 
@@ -320,8 +300,7 @@ class Queue:
     """
       Get priority from this queue.
       Lower number means higher priority value.
-      Legal value range is [1, 6].
+      Legal value range is [-128, 127].
       Values out of this range might work, but are non-standard.
     """
-    return 6
-
+    return 128

product/CMFActivity/ActivityTool.py

@@ -962,25 +962,26 @@ class ActivityTool (Folder, UniqueObject):
       inner_self = aq_inner(self)
 
       try:
-        #Sort activity list by priority
-        activity_list = sorted(activity_dict.itervalues(),
-                               key=lambda activity: activity.getPriority(self))
-
-        # Wakeup each queue
-        for activity in activity_list:
-          activity.wakeup(inner_self, processing_node)
-
-        # Process messages on each queue in round robin
-        has_awake_activity = 1
-        while has_awake_activity:
-          has_awake_activity = 0
-          for activity in activity_list:
-            if is_running_lock.acquire(0):
-              try:
-                activity.tic(inner_self, processing_node) # Transaction processing is the responsability of the activity
-                has_awake_activity = has_awake_activity or activity.isAwake(inner_self, processing_node)
-              finally:
-                is_running_lock.release()
+        # Loop as long as there are activities. Always process the queue with
+        # "highest" priority. If several queues have same highest priority, do
+        # not choose one that has just been processed.
+        # This algorithm is fair enough because we actually use only 2 queues.
+        # Otherwise, a round-robin of highest-priority queues would be required.
+        # XXX: We always finish by iterating over all queues, in case that
+        #      getPriority does not see messages dequeueMessage would process.
+        last = None
+        def sort_key(activity):
+          return activity.getPriority(self), activity is last
+        while is_running_lock.acquire(0):
+          try:
+            for last in sorted(activity_dict.itervalues(), key=sort_key):
+              # Transaction processing is the responsability of the activity
+              if not last.dequeueMessage(inner_self, processing_node):
+                break
+            else:
+              break
+          finally:
+            is_running_lock.release()
       finally:
         # decrease the number of active_threads
         tic_lock.acquire()

product/CMFActivity/tests/testCMFActivity.py

@@ -3184,16 +3184,16 @@ class TestCMFActivity(ERP5TypeTestCase, LogInterceptor):
       rendez_vous_event.set()
       # When this event is available, it means test has called process_shutdown.
       activity_event.wait()
-    from Products.CMFActivity.Activity.Queue import Queue
-    original_queue_tic = Queue.tic
+    from Products.CMFActivity.Activity.SQLDict import SQLDict
+    original_dequeue = SQLDict.dequeueMessage
     queue_tic_test_dict = {}
-    def Queue_tic(self, activity_tool, processing_node):
-      result = original_queue_tic(self, activity_tool, processing_node)
-      queue_tic_test_dict['isAlive'] = process_shutdown_thread.isAlive()
+    def dequeueMessage(self, activity_tool, processing_node):
       # This is a one-shot method, revert after execution
-      Queue.tic = original_queue_tic
+      SQLDict.dequeueMessage = original_dequeue
+      result = self.dequeueMessage(activity_tool, processing_node)
+      queue_tic_test_dict['isAlive'] = process_shutdown_thread.isAlive()
       return result
-    Queue.tic = Queue_tic
+    SQLDict.dequeueMessage = dequeueMessage
     Organisation.waitingActivity = waitingActivity
     try:
       # Use SQLDict with no group method so that both activities won't be
@@ -3258,7 +3258,7 @@ class TestCMFActivity(ERP5TypeTestCase, LogInterceptor):
           pass
     finally:
       delattr(Organisation, 'waitingActivity')
-      Queue.tic = original_queue_tic
+      SQLDict.dequeueMessage = original_dequeue
 
   def test_hasActivity(self):
     active_object = self.portal.organisation_module.newContent(