Skip to content

Commit

Permalink
migration: provide default copy behavior per #7623
Browse files Browse the repository at this point in the history
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<PoolManagerGetPoolsByPoolGroupOfPoolMessage> implements RefreshablePoolList`
  * New `messageArrived(PoolManagerGetPoolsByPoolGroupOfPoolMessage msg)` overload for `PoolManagerV5`.
* Change history:
  * Implement #7623

  * Implement logic described by #7623 (comment)

  * 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
  • Loading branch information
greenc-FNAL committed Sep 25, 2024
1 parent be9d1c7 commit cfeed0c
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -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<String, List<PoolManagerPoolInformation>> _poolsMap;
private Collection<String> _offlinePools;

public PoolManagerGetPoolsByPoolGroupOfPoolMessage(String poolName) {
super(true);
_poolName = requireNonNull(poolName);
}

public String getPoolName() {
return _poolName;
}

public Map<String, List<PoolManagerPoolInformation>> getPoolsMap() {
return _poolsMap;
}

public void setPoolsMap(Map<String, List<PoolManagerPoolInformation>> poolsMap) {
_poolsMap = new HashMap<>(poolsMap);
}

public Collection<String> getOfflinePools() {
return (_offlinePools == null) ? Collections.emptyList() : _offlinePools;
}

public void setOfflinePools(Collection<String> _offlinePools) {
this._offlinePools = _offlinePools;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -524,6 +526,30 @@ private void getPoolInformation(
return msg;
}

public PoolManagerGetPoolsByPoolGroupOfPoolMessage
messageArrived(PoolManagerGetPoolsByPoolGroupOfPoolMessage msg) {
try {
Collection<PoolSelectionUnit.SelectionPoolGroup> poolGroups =
_selectionUnit.getPoolGroupsOfPool(msg.getPoolName());
Map<String, List<PoolManagerPoolInformation>> poolsMap = new HashMap<>();
List<String> offlinePools = new ArrayList<>();
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) {
Map<String, List<PoolManagerPoolInformation>> empty = Map.of();
msg.setPoolsMap(empty);
msg.setSucceeded();
}
return msg;
}

public PoolManagerGetPoolsByPoolGroupMessage
messageArrived(PoolManagerGetPoolsByPoolGroupMessage msg) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -537,25 +537,32 @@ public class MigrationCopyCommand implements Callable<String> {
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<String> 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));
Expand Down Expand Up @@ -801,8 +808,14 @@ public String call() throws IllegalArgumentException {
Expression includeExpression =
createPoolPredicate(includeWhen, TRUE_EXPRESSION);

List<String> 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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<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(
_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);
}
}

0 comments on commit cfeed0c

Please sign in to comment.