From cfeed0cc3ce399056472b5361b19dee433b0de4a Mon Sep 17 00:00:00 2001 From: Chris Green Date: Fri, 6 Sep 2024 13:33:08 -0500 Subject: [PATCH] migration: provide default copy behavior per #7623 When `migration copy -pnfsid=X -target=pgroup` is executed without specification of a pool group, identify and use the pool group to which the source's pool(s) belong, if this is singular. Motivation: More intuitive implementation of a common use case. Modules: dcache dcache-vehicles Modification: * Change summary: * `MigrationModule` now allows empty target specification for `migration copy` IFF `-target=pgroup`. * New message `class PoolManagerGetPoolsByPoolGroupOfPoolMessage extends Message`. * New `class PoolListByPoolGroupOfPool extends AbstractMessageCallback implements RefreshablePoolList` * New `messageArrived(PoolManagerGetPoolsByPoolGroupOfPoolMessage msg)` overload for `PoolManagerV5`. * Change history: * Implement #7623 * Implement logic described by https://github.com/dCache/dcache/issues/7623#issuecomment-2278266546 * Fix handling of target list to account for possible omission * Generalize by renaming, per @DmitryLitvintsev suggestion * `PoolManagerGetPoolsBySourcePoolPoolGroupMessage` -> `PoolManagerGetPoolsByPoolGroupOfPoolMessage` * `PoolListBySourcePoolPoolGroup` -> `PoolListByPoolGroupOfPool` * `getPoolsBySourcePoolPoolGroup()` -> `getPoolsByPoolGroupOfPool()` * `(_?)sourcePool` -> `\1poolName` * `getSourcePool()` -> `getPoolName()` * "source pool" -> "pool" * 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. * Address https://rb.dcache.org/r/14316/#comment29388 * Fix imports per https://rb.dcache.org/r/14316/#comment29389 * Reformat new code per official DCache style * Fix variable name per https://rb.dcache.org/r/14316/#comment29392 Result: `migration copy -pnfsid=X -target=pgroup` successfully identifies and uses an appropriate default pool group, where a unique one exists. Release notes: Pool migration : use pool group of source as default destination pool group on copy with `-target=pgroup` when identifiable and unique. Target: master Request: Patch: https://rb.dcache.org/r/14316/ Requires-notes: yes Requires-book: no Acked-by: Tigran Mkrtchyan, Dmitry Litvintsev --- ...nagerGetPoolsByPoolGroupOfPoolMessage.java | 44 +++++++++++ .../poolManager/PoolManagerV5.java | 26 +++++++ .../pool/migration/MigrationModule.java | 25 ++++-- .../migration/PoolListByPoolGroupOfPool.java | 76 +++++++++++++++++++ 4 files changed, 165 insertions(+), 6 deletions(-) create mode 100644 modules/dcache-vehicles/src/main/java/diskCacheV111/vehicles/PoolManagerGetPoolsByPoolGroupOfPoolMessage.java create mode 100644 modules/dcache/src/main/java/org/dcache/pool/migration/PoolListByPoolGroupOfPool.java diff --git a/modules/dcache-vehicles/src/main/java/diskCacheV111/vehicles/PoolManagerGetPoolsByPoolGroupOfPoolMessage.java b/modules/dcache-vehicles/src/main/java/diskCacheV111/vehicles/PoolManagerGetPoolsByPoolGroupOfPoolMessage.java new file mode 100644 index 00000000000..2b77cd0a536 --- /dev/null +++ b/modules/dcache-vehicles/src/main/java/diskCacheV111/vehicles/PoolManagerGetPoolsByPoolGroupOfPoolMessage.java @@ -0,0 +1,44 @@ +package diskCacheV111.vehicles; + +import static java.util.Objects.requireNonNull; + +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class PoolManagerGetPoolsByPoolGroupOfPoolMessage + extends Message { + + private static final long serialVersionUID = -4022990392097610436L; + + private final String _poolName; + private Map> _poolsMap; + private Collection _offlinePools; + + public PoolManagerGetPoolsByPoolGroupOfPoolMessage(String poolName) { + super(true); + _poolName = requireNonNull(poolName); + } + + public String getPoolName() { + return _poolName; + } + + public Map> getPoolsMap() { + return _poolsMap; + } + + public void setPoolsMap(Map> poolsMap) { + _poolsMap = new HashMap<>(poolsMap); + } + + public Collection getOfflinePools() { + return (_offlinePools == null) ? Collections.emptyList() : _offlinePools; + } + + public void setOfflinePools(Collection _offlinePools) { + this._offlinePools = _offlinePools; + } +} diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolManagerV5.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolManagerV5.java index 33441a26888..900eb68065e 100644 --- a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolManagerV5.java +++ b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolManagerV5.java @@ -18,6 +18,7 @@ import diskCacheV111.vehicles.PoolManagerGetPoolsByLinkMessage; import diskCacheV111.vehicles.PoolManagerGetPoolsByNameMessage; import diskCacheV111.vehicles.PoolManagerGetPoolsByPoolGroupMessage; +import diskCacheV111.vehicles.PoolManagerGetPoolsByPoolGroupOfPoolMessage; import diskCacheV111.vehicles.PoolManagerPoolInformation; import diskCacheV111.vehicles.PoolManagerPoolModeMessage; import diskCacheV111.vehicles.PoolManagerPoolUpMessage; @@ -42,6 +43,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; @@ -524,6 +526,30 @@ private void getPoolInformation( return msg; } + public PoolManagerGetPoolsByPoolGroupOfPoolMessage + messageArrived(PoolManagerGetPoolsByPoolGroupOfPoolMessage msg) { + try { + Collection poolGroups = + _selectionUnit.getPoolGroupsOfPool(msg.getPoolName()); + Map> poolsMap = new HashMap<>(); + List offlinePools = new ArrayList<>(); + for (PoolSelectionUnit.SelectionPoolGroup poolGroup : poolGroups) { + List 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) { + Map> empty = Map.of(); + msg.setPoolsMap(empty); + msg.setSucceeded(); + } + return msg; + } + public PoolManagerGetPoolsByPoolGroupMessage messageArrived(PoolManagerGetPoolsByPoolGroupMessage msg) { try { diff --git a/modules/dcache/src/main/java/org/dcache/pool/migration/MigrationModule.java b/modules/dcache/src/main/java/org/dcache/pool/migration/MigrationModule.java index 7f6ff57045c..db419a5cdea 100644 --- a/modules/dcache/src/main/java/org/dcache/pool/migration/MigrationModule.java +++ b/modules/dcache/src/main/java/org/dcache/pool/migration/MigrationModule.java @@ -537,25 +537,32 @@ public class MigrationCopyCommand implements Callable { usage = "Enables the transfer of files from a disabled pool.") boolean forceSourceMode; - @Argument(metaVar = "target") - String[] targets; + @Argument(metaVar = "target", + required = false, + usage = "Required unless -target=pgroup is supplied, in which case we" + + "default to the unique pool group of which the source pool is a member," + + "if such exists.") + String[] targets = {}; @CommandLine String commandLine; private RefreshablePoolList createPoolList(String type, List targets) { CellStub poolManager = _context.getPoolManagerStub(); - switch (type) { case "pool": return new PoolListByNames(poolManager, targets); case "hsm": return new PoolListByHsm(poolManager, targets); case "pgroup": - return new PoolListByPoolGroup(poolManager, targets); + if (targets.isEmpty()) { + return new PoolListByPoolGroupOfPool(poolManager, _context.getPoolName()); + } else { + return new PoolListByPoolGroup(poolManager, targets); + } case "link": if (targets.size() != 1) { - throw new IllegalArgumentException(targets.toString() + + throw new IllegalArgumentException(targets + ": Only one target supported for -type=link"); } return new PoolListByLink(poolManager, targets.get(0)); @@ -801,8 +808,14 @@ public String call() throws IllegalArgumentException { Expression includeExpression = createPoolPredicate(includeWhen, TRUE_EXPRESSION); + List listOfTargets = asList(targets); + if (listOfTargets.isEmpty() && !target.equals("pgroup")) { + throw new IllegalArgumentException( + "target(s) required when target type is " + target); + } + RefreshablePoolList poolList = - new PoolListFilter(createPoolList(target, asList(targets)), + new PoolListFilter(createPoolList(target, listOfTargets), excluded, excludeExpression, included, includeExpression, sourceList); 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 new file mode 100644 index 00000000000..e904209bad1 --- /dev/null +++ b/modules/dcache/src/main/java/org/dcache/pool/migration/PoolListByPoolGroupOfPool.java @@ -0,0 +1,76 @@ +package org.dcache.pool.migration; + +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.PoolManagerGetPoolsByPoolGroupOfPoolMessage; +import diskCacheV111.vehicles.PoolManagerPoolInformation; +import java.util.List; +import org.dcache.cells.AbstractMessageCallback; +import org.dcache.cells.CellStub; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class PoolListByPoolGroupOfPool + extends AbstractMessageCallback + implements RefreshablePoolList { + + private static final Logger LOGGER = + LoggerFactory.getLogger(PoolListByPoolGroupOfPool.class); + + private final CellStub _poolManager; + private final String _poolName; + private ImmutableMap> _poolsMap; + private ImmutableList _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 getOfflinePools() { + return _offlinePools; + } + + @Override + public ImmutableList getPools() { + return _poolsMap.isEmpty() ? + ImmutableList.of() : + ImmutableList.copyOf(_poolsMap.values().iterator().next()); + } + + @Override + public void refresh() { + CellStub.addCallback( + _poolManager.send(new PoolManagerGetPoolsByPoolGroupOfPoolMessage(_poolName)), + this, MoreExecutors.directExecutor()); + } + + @Override + public String toString() { + return String.format("source pool %s, %d pools", + _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); + } +}