for read- /write-pool selection fall back to no zone filtering levels again if no pool with correct zone was found in first go

Author: lens161

modules/dcache/src/main/java/diskCacheV111/poolManager/PoolMonitorV5.java

@@ -252,20 +252,45 @@ public SelectedPool selectWritePool(long preallocated)
                             .toLowerCase()));
             }
 
+            if (_zone.isPresent()) {
+                SelectedPool pool = filterWritePool(levels, true, preallocated);
+                if (pool != null) {
+                    return pool;
+                }
+            }
+
+            SelectedPool pool = filterWritePool(levels, false, preallocated);
+            if (pool != null) {
+                return pool;
+            }
+
+            throw new CacheException(CacheException.NO_POOL_ONLINE,
+                  noOnlinePoolsErrorMessage(DirectionType.WRITE.name()
+                        .toLowerCase()));
+        }
+
+       @Nullable
+        private SelectedPool filterWritePool(PoolPreferenceLevel[] levels,
+                                           boolean filterByZone,
+                                           long preallocated) throws CacheException{
             CostException fallback = null;
+
             for (PoolPreferenceLevel level : levels) {
                 List<PoolInfo> pools =
-                      level.getPoolList().stream()
-                            .map(_costModule::getPoolInfo)
-                            .filter(Objects::nonNull)
-                            .collect(toList());
-                pools = filterByZone(pools);
+                        level.getPoolList().stream()
+                                .map(_costModule::getPoolInfo)
+                                .filter(Objects::nonNull)
+                                .collect(toList());
+
+                if (filterByZone){
+                    pools = filterByZone(pools);
+                }
 
                 if (!pools.isEmpty()) {
                     Partition partition = _partitionManager.getPartition(level.getTag());
                     try {
                         return partition.selectWritePool(_costModule, pools, _fileAttributes,
-                              preallocated);
+                                preallocated);
                     } catch (CostException e) {
                         if (!e.shouldFallBack()) {
                             throw e;
@@ -275,16 +300,11 @@ public SelectedPool selectWritePool(long preallocated)
                 }
             }
 
-            /* We were asked to fall back, but all available links were
-             * exhausted. Let the caller deal with it.
-             */
             if (fallback != null) {
                 throw fallback;
             }
 
-            throw new CacheException(CacheException.NO_POOL_ONLINE,
-                  noOnlinePoolsErrorMessage(DirectionType.WRITE.name()
-                        .toLowerCase()));
+            return null;
         }
 
         @Override
@@ -318,12 +338,37 @@ public SelectedPool selectReadPool()
                             .toLowerCase()));
             }
 
+
+            if (_zone.isPresent()) {
+                SelectedPool pool = filterReadPool(level, onlinePoolsWithFile, true);
+                if (pool != null) {
+                    return pool;
+                }
+            }
+
+            SelectedPool pool = filterReadPool(level, onlinePoolsWithFile, false);
+            if (pool != null) {
+                return pool;
+            }
+
+            /* None of the pools we were allowed to read from were
+             * online or had the file.
+             */
+            throw new PermissionDeniedCacheException(
+                  "File is online, but not in read-allowed pool");
+        }
+
+        @Nullable
+        private SelectedPool filterReadPool(PoolPreferenceLevel[] level,
+                                  Map<String, PoolInfo> onlinePoolsWithFile,
+                                  boolean filterByZone) throws CacheException {
+
             CostException costException = null;
 
             for (int prio = 0; prio < level.length; prio++) {
                 List<String> poolsInCurrentLevel = level[prio].getPoolList();
                 LOGGER.debug("[read] Allowed pools at level {}: {}",
-                      prio, poolsInCurrentLevel);
+                        prio, poolsInCurrentLevel);
 
                 if (poolsInCurrentLevel.isEmpty()) {
                     // No pools in this level....skip it.
@@ -341,9 +386,11 @@ public SelectedPool selectReadPool()
                     }
                 }
                 LOGGER.debug("[read] Available pools at level {}: {}",
-                      prio, pools);
+                        prio, pools);
 
-                pools = filterByZone(pools);
+                if (filterByZone) {
+                    pools = filterByZone(pools);
+                }
 
                 /* If allowed, fallback to next link if current link doesn't point
                  * to any pool holding the file.
@@ -360,14 +407,14 @@ public SelectedPool selectReadPool()
                  * to select a pool.
                  */
                 _partition =
-                      _partitionManager.getPartition(level[prio].getTag());
+                        _partitionManager.getPartition(level[prio].getTag());
 
                 /* The actual pool selection is delegated to the
                  * Partition.
                  */
                 try {
                     return _partition.selectReadPool(_costModule, pools,
-                          _fileAttributes);
+                            _fileAttributes);
                 } catch (CostException e) {
                     costException = e;
                     if (!e.shouldFallBack()) {
@@ -376,35 +423,21 @@ public SelectedPool selectReadPool()
                 }
             }
 
-            /* If we have a CostException where a pool was selected and
-             * shouldTryAlternatives not set then we return that pool anyway.
-             * REVISIT: consider updating partitions so they don't throw
-             * an exception in this case.
-             */
             if (costException != null) {
                 if (costException.getPool() != null
-                      && !costException.shouldTryAlternatives()) {
+                        && !costException.shouldTryAlternatives()) {
                     return costException.getPool();
                 }
-
                 throw costException;
             }
 
-            /* None of the pools we were allowed to read from were
-             * online or had the file.
-             */
-            throw new PermissionDeniedCacheException(
-                  "File is online, but not in read-allowed pool");
+            return null;
         }
 
         private List<PoolInfo> filterByZone(List<PoolInfo> pools) {
-            if (_zone.isPresent()) {
-                List<PoolInfo> poolsZ =
-                        pools.stream()
+                pools = pools.stream()
                         .filter(p -> Objects.equals(p.getTags().get("zone"), _zone.get()))
                         .toList();
-                pools = (poolsZ.isEmpty()) ? pools : poolsZ;
-            }
             return pools;
         }
 

modules/dcache/src/test/java/org/dcache/tests/poolmanager/PoolMonitorTest.java

@@ -179,11 +179,47 @@ public void testReadPoolZonePreference() throws Exception {
         assertEquals("pool1", selector.selectReadPool().name());
     }
 
+    @Test
+    public void testReadPoolZoneFallback() throws Exception {
+        prepareCostModule(false, true);
+
+        // Only pool2 in zone 2 has the file
+        FileAttributes attributes = FileAttributes.of()
+                .pnfsId(_pnfsId)
+                .locations(Collections.singleton("pool2"))
+                .build();
+        StorageInfos.injectInto(_storageInfo, attributes);
+
+        PoolSelector selector = _poolMonitor.getPoolSelector(attributes, _protocolInfo,
+                null, Optional.of("1"), Collections.EMPTY_SET);
+
+        assertEquals("pool2", selector.selectReadPool().name());
+    }
+
+    @Test
+    public void testWritePoolZoneFallback() throws Exception {
+        // pool2 is offline zone 2 requested so falls back to pool1
+        prepareCostModule(false, true, true);
+
+        FileAttributes attributes = FileAttributes.of().pnfsId(_pnfsId).build();
+        StorageInfos.injectInto(_storageInfo, attributes);
+
+        PoolSelector selector = _poolMonitor.getPoolSelector(attributes, _protocolInfo,
+                null, Optional.of("2"), Collections.EMPTY_SET);
+
+        assertEquals("pool1", selector.selectWritePool(0).name());
+    }
+
     private void prepareCostModule(boolean linkPerPool) throws Exception {
         prepareCostModule(linkPerPool, false);
     }
 
     private void prepareCostModule(boolean linkPerPool, boolean withZones) throws Exception {
+        prepareCostModule(linkPerPool, withZones, false);
+    }
+
+    private void prepareCostModule(boolean linkPerPool, boolean withZones, boolean pool2Offline)
+            throws Exception {
         if (linkPerPool) {
             PoolMonitorHelper.prepareLinkPerPool(_selectionUnit, _access, _pools);
         } else {
@@ -198,31 +234,33 @@ private void prepareCostModule(boolean linkPerPool, boolean withZones) throws Ex
         PoolV2Mode poolMode = new PoolV2Mode(PoolV2Mode.ENABLED);
 
         PoolCostInfo poolCost1 = new PoolCostInfo("pool1", IoQueueManager.DEFAULT_QUEUE);
-        PoolCostInfo poolCost2 = new PoolCostInfo("pool2", IoQueueManager.DEFAULT_QUEUE);
-
         poolCost1.setSpaceUsage(100, 20, 30, 50);
-        poolCost2.setSpaceUsage(100, 20, 30, 50);
 
         PoolManagerPoolUpMessage pool1UpMessage = new PoolManagerPoolUpMessage("pool1",
               serialId, poolMode, poolCost1);
-        PoolManagerPoolUpMessage pool2UpMessage = new PoolManagerPoolUpMessage("pool2",
-              serialId, poolMode, poolCost2);
-
         pool1UpMessage.setHostName(_localhost);
-        pool2UpMessage.setHostName(_localhost);
 
         if (withZones) {
             pool1UpMessage.setTagMap(Map.of("zone", "1"));
-            pool2UpMessage.setTagMap(Map.of("zone", "2"));
         }
 
         CellMessage envelope1 = new CellMessage(new CellAddressCore("PoolManager"), null);
         envelope1.addSourceAddress(new CellAddressCore("pool1"));
-        CellMessage envelope2 = new CellMessage(new CellAddressCore("PoolManager"), null);
-        envelope2.addSourceAddress(new CellAddressCore("pool2"));
-
         _costModule.messageArrived(envelope1, pool1UpMessage);
-        _costModule.messageArrived(envelope2, pool2UpMessage);
+
+        if (!pool2Offline) {
+            PoolCostInfo poolCost2 = new PoolCostInfo("pool2", IoQueueManager.DEFAULT_QUEUE);
+            poolCost2.setSpaceUsage(100, 20, 30, 50);
+            PoolManagerPoolUpMessage pool2UpMessage = new PoolManagerPoolUpMessage("pool2",
+                  serialId, poolMode, poolCost2);
+            pool2UpMessage.setHostName(_localhost);
+            if (withZones) {
+                pool2UpMessage.setTagMap(Map.of("zone", "2"));
+            }
+            CellMessage envelope2 = new CellMessage(new CellAddressCore("PoolManager"), null);
+            envelope2.addSourceAddress(new CellAddressCore("pool2"));
+            _costModule.messageArrived(envelope2, pool2UpMessage);
+        }
     }
 
     private PoolSelector prepareHostExclusion() throws Exception {