Merge pull request #4551 from bziemons/patch-4546-sort_replicas_proto…

…col_priority

Replicas: Always sort replicas by priority; Fix #4546

lib/rucio/core/replica_sorter.py

@@ -33,6 +33,7 @@
 import socket
 import tarfile
 import time
+from collections import OrderedDict
 from math import asin, cos, radians, sin, sqrt
 from typing import TYPE_CHECKING
 from urllib.parse import urlparse
@@ -179,9 +180,19 @@ def sort_replicas(dictreplica: "Dict", client_location: "Dict", selection: "Opti
     :param default: the default sorting algorithm (random, if not defined).
     :returns: the keys of dictreplica in a sorted list.
     """
+    if len(dictreplica) == 0:
+        return []
     if not selection:
         selection = 'geoip'
 
+    items = [(key, value) for key, value in dictreplica.items()]
+    # safety check, TODO: remove if all dictreplica values are 4-tuple with priority as second item
+    if isinstance(items[0][1], tuple) and len(items[0][1]) == 4:
+        # sort by value[1], which is the priority
+        items.sort(key=lambda item: item[1][1])
+    dictreplica = OrderedDict(items)
+
+    # all sorts must be stable to preserve the priority (the Python standard sorting functions always are stable)
     if selection == 'geoip':
         replicas = sort_geoip(dictreplica, client_location, ignore_error=True)
     elif selection == 'closeness':

lib/rucio/web/rest/flaskapi/v1/replicas.py

@@ -134,10 +134,8 @@ def generate(vo):
                         yield f'  <glfn name="/atlas/rucio/{rfile["scope"]}:{rfile["name"]}">'
                         yield '</glfn>\n'
 
-                        idx = 0
-                        for replica in replicas:
-                            yield '   <url location="' + str(dictreplica[replica]) + '" priority="' + str(idx + 1) + '">' + escape(replica) + '</url>\n'
-                            idx += 1
+                        for idx, replica in enumerate(replicas, start=1):
+                            yield '   <url location="' + str(dictreplica[replica]) + '" priority="' + str(idx) + '">' + escape(replica) + '</url>\n'
                             if limit and limit == idx:
                                 break
                         yield ' </file>\n'
@@ -389,18 +387,18 @@ def generate(request_id, issuer, vo):
                         yield f'  <glfn name="/{policy_schema}/rucio/{rfile["scope"]}:{rfile["name"]}"></glfn>\n'
 
                         lanreplicas = [replica for replica, v in dictreplica.items() if v[0] == 'lan']
+                        # sort lan by priority
+                        lanreplicas.sort(key=lambda rep: dictreplica[rep][1])
                         replicas = lanreplicas + sort_replicas({k: v for k, v in dictreplica.items() if v[0] != 'lan'}, client_location, selection=select)
 
-                        idx = 1
-                        for replica in replicas:
+                        for idx, replica in enumerate(replicas, start=1):
                             yield '  <url location="' + str(dictreplica[replica][2]) \
                                 + '" domain="' + str(dictreplica[replica][0]) \
                                 + '" priority="' + str(idx) \
                                 + '" client_extract="' + str(dictreplica[replica][3]).lower() \
                                 + '">' + escape(replica) + '</url>\n'
                             if limit and limit == idx:
                                 break
-                            idx += 1
                         yield ' </file>\n'
 
                 if metalink:

lib/rucio/web/rest/webpy/v1/replica.py

@@ -460,6 +460,8 @@ def POST(self):
                                                                         rfile['name'])
 
                     lanreplicas = [replica for replica, v in dictreplica.items() if v[0] == 'lan']
+                    # sort lan by priority
+                    lanreplicas.sort(key=lambda rep: dictreplica[rep][1])
                     replicas = lanreplicas + sort_replicas({k: v for k, v in dictreplica.items() if v[0] != 'lan'}, client_location, selection=select)
 
                     idx = 1