Move check of number of pool groups returned from server to client

Per https://rb.dcache.org/r/14316/#comment29381 N.B. The MigrationModule itself appears to have no access to the `poolList` upon job completion to verify its parameters, so the size check on the map is currently implemented in `PoolListByPoolGroupOfPool.success()`. Please advise if this is incorrect.
dCache · Sep 17, 2024 · eafa09c · eafa09c
1 parent 8f6e24a
commit eafa09c
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 46 deletions.
diff --git a/modules/dcache-resilience/src/test/java/org/dcache/resilience/TestSelectionUnit.java b/modules/dcache-resilience/src/test/java/org/dcache/resilience/TestSelectionUnit.java
@@ -169,11 +169,6 @@ public Collection<SelectionPool> getPoolsByPoolGroup(String poolGroup)
         return psu.getPoolsByPoolGroup(poolGroup);
     }
 
-    @Override
-    public Collection<SelectionPool> getPoolsByPoolGroupOfPool(String poolName) throws NoSuchElementException {
-        return psu.getPoolsByPoolGroupOfPool(poolName);
-    }
-
     @Override
     public String getProtocolUnit(String protocolUnitName) {
         return psu.getProtocolUnit(protocolUnitName);

diff --git a/...les/src/main/java/diskCacheV111/vehicles/PoolManagerGetPoolsByPoolGroupOfPoolMessage.java b/...les/src/main/java/diskCacheV111/vehicles/PoolManagerGetPoolsByPoolGroupOfPoolMessage.java
@@ -1,19 +1,44 @@
 package diskCacheV111.vehicles;
 
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 import static java.util.Objects.requireNonNull;
 
 public class PoolManagerGetPoolsByPoolGroupOfPoolMessage
-      extends PoolManagerGetPoolsMessage {
+      extends Message {
 
-    private static final long serialVersionUID = 4423670920097918847L;
+    private static final long serialVersionUID = -4022990392097610436L;
 
     private final String _poolName;
+    private Map<String, List<PoolManagerPoolInformation>> _poolsMap;
+    private Collection<String> _offlinePools;
 
     public PoolManagerGetPoolsByPoolGroupOfPoolMessage(String poolName) {
+        super(true);
         _poolName = requireNonNull(poolName);
     }
 
     public String getPoolName() {
         return _poolName;
     }
+
+    public void setPoolsMap(Map<String, List<PoolManagerPoolInformation>> poolsMap) {
+        _poolsMap = new HashMap<>(poolsMap);
+    }
+
+    public void setOfflinePools(Collection<String> _offlinePools) {
+        this._offlinePools = _offlinePools;
+    }
+
+    public Map<String, List<PoolManagerPoolInformation>> getPoolsMap() {
+        return _poolsMap;
+    }
+
+    public Collection<String> getOfflinePools() {
+        return (_offlinePools == null) ? Collections.emptyList() : _offlinePools;
+    }
 }
diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolManagerV5.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolManagerV5.java
@@ -39,15 +39,7 @@
 import dmg.cells.nucleus.DelayedReply;
 import java.io.PrintWriter;
 import java.net.InetSocketAddress;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.NoSuchElementException;
-import java.util.Set;
-import java.util.StringTokenizer;
+import java.util.*;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
@@ -528,17 +520,22 @@ private void getPoolInformation(
     public PoolManagerGetPoolsByPoolGroupOfPoolMessage
     messageArrived(PoolManagerGetPoolsByPoolGroupOfPoolMessage msg) {
         try {
-            List<PoolManagerPoolInformation> pools = new ArrayList<>();
+            Collection<PoolSelectionUnit.SelectionPoolGroup> poolGroups =
+                    _selectionUnit.getPoolGroupsOfPool(msg.getPoolName());
+            Map<String, List<PoolManagerPoolInformation>> poolsMap = new HashMap<>();
             List<String> offlinePools = new ArrayList<>();
-            getPoolInformation(_selectionUnit.getPoolsByPoolGroupOfPool(msg.getPoolName()), pools,
-                      offlinePools);
-            msg.setPools(pools);
+            for (PoolSelectionUnit.SelectionPoolGroup poolGroup : poolGroups) {
+                List<PoolManagerPoolInformation> pools = new ArrayList<>();
+                getPoolInformation(_selectionUnit.getPoolsByPoolGroup(poolGroup.getName()),
+                        pools, offlinePools);
+                poolsMap.put(poolGroup.getName(), pools);
+            }
+            msg.setPoolsMap(poolsMap);
             msg.setOfflinePools(offlinePools);
             msg.setSucceeded();
         } catch (NoSuchElementException e) {
-            Collection<PoolManagerPoolInformation> empty =
-                  Collections.emptyList();
-            msg.setPools(empty);
+            Map<String, List<PoolManagerPoolInformation>> empty = new HashMap<>();
+            msg.setPoolsMap(empty);
             msg.setSucceeded();
         }
         return msg;

diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnit.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnit.java
@@ -282,8 +282,6 @@ boolean updatePool(String poolName, CellAddressCore address, String canonicalHos
 
     Collection<SelectionPool> getPoolsByPoolGroup(String poolGroup) throws NoSuchElementException;
 
-    Collection<SelectionPool> getPoolsByPoolGroupOfPool(String poolName) throws NoSuchElementException;
-
     Collection<SelectionPool> getAllDefinedPools(boolean enabledOnly);
 
     Collection<SelectionPoolGroup> getPoolGroupsOfPool(String PoolName);

diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java
@@ -2622,25 +2622,6 @@ public Collection<SelectionPool> getPoolsByPoolGroup(String poolGroup)
         return new ArrayList<>(group._poolList.values());
     }
 
-    @Override
-    public Collection<SelectionPool> getPoolsByPoolGroupOfPool(String poolName)
-          throws NoSuchElementException {
-        Collection<SelectionPoolGroup> groups;
-        try {
-            groups = getPoolGroupsOfPool(poolName);
-        } catch (NoSuchElementException e) {
-            throw new NoSuchElementException("No pool group for nonexistent pool: " + poolName);
-        }
-
-        if (groups.isEmpty()) {
-            throw new NoSuchElementException("No pool group for pool: " + poolName);
-        } else if (groups.size() > 1) {
-            throw new NoSuchElementException("No unique pool group for pool: " + poolName);
-        }
-
-        return new ArrayList<>(groups.iterator().next().getPools());
-    }
-
     @Override
     public Collection<SelectionPool> getAllDefinedPools(boolean enabledOnly) {
         List<SelectionPool> pools = new ArrayList<>(_pools.size());

diff --git a/modules/dcache/src/main/java/org/dcache/pool/migration/PoolListByPoolGroupOfPool.java b/modules/dcache/src/main/java/org/dcache/pool/migration/PoolListByPoolGroupOfPool.java
@@ -2,21 +2,55 @@
 
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.util.concurrent.MoreExecutors;
+import diskCacheV111.vehicles.Pool;
 import diskCacheV111.vehicles.PoolManagerGetPoolsByPoolGroupOfPoolMessage;
+import diskCacheV111.vehicles.PoolManagerPoolInformation;
+import org.dcache.cells.AbstractMessageCallback;
 import org.dcache.cells.CellStub;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
 
 class PoolListByPoolGroupOfPool
-      extends PoolListFromPoolManager {
+        extends AbstractMessageCallback<PoolManagerGetPoolsByPoolGroupOfPoolMessage>
+        implements RefreshablePoolList {
+
+    private static final Logger LOGGER =
+            LoggerFactory.getLogger(PoolListByPoolGroupOfPool.class);
 
     private final CellStub _poolManager;
     private final String _poolName;
+    private ImmutableMap<String, List<PoolManagerPoolInformation>> _poolsMap;
+    private ImmutableList<String> _offlinePools = ImmutableList.of();
+
+    private boolean _isValid;
 
     public PoolListByPoolGroupOfPool(CellStub poolManager, String poolName) {
         _poolManager = requireNonNull(poolManager);
         _poolName = requireNonNull(poolName);
     }
 
+    @Override
+    public synchronized boolean isValid() {
+        return _isValid;
+    }
+
+    @Override
+    public ImmutableList<String> getOfflinePools() {
+        return _offlinePools;
+    }
+
+    @Override
+    public ImmutableList<PoolManagerPoolInformation> getPools() {
+        return _poolsMap.isEmpty() ?
+                ImmutableList.of() :
+                ImmutableList.copyOf(_poolsMap.values().iterator().next());
+    }
+
     @Override
     public void refresh() {
         CellStub.addCallback(
@@ -27,6 +61,18 @@ public void refresh() {
     @Override
     public String toString() {
         return String.format("source pool %s, %d pools",
-                _poolName, _pools.size());
+                _poolName, getPools().size());
+    }
+
+    @Override
+    public void success(PoolManagerGetPoolsByPoolGroupOfPoolMessage message) {
+        _poolsMap = ImmutableMap.copyOf(message.getPoolsMap());
+        _offlinePools = ImmutableList.copyOf(message.getOfflinePools());
+        _isValid = (_poolsMap.size() == 1);
+    }
+
+    @Override
+    public void failure(int rc, Object error) {
+        LOGGER.error("Failed to query pool manager ({})", error);
     }
 }