From fd2b56f63762ae3acd5ead822d6810eb06ffaec8 Mon Sep 17 00:00:00 2001 From: Tigran Mkrtchyan Date: Thu, 23 Jun 2022 10:11:11 +0200 Subject: [PATCH] poolmanager: add possibility to create nested pool groups. Motivation: Many sites have the setup like following: poolA-1,poolA-2,...poolZ-1,poolZ-2 pgroup-odd: poolA-1,...poolZ-1 pgroup-even: poolA-2,..poolZ-2 group-all: poolA-1,poolA-2,...poolZ-1,poolZ-2 The manual adding to all those pool groups might (and does) endup with invalid configuration. Moreover, this is not possible with dynamic groups. The configuration issue can be addressed by nested pool groups, which are identified by special prefix '@': i``` psu create pgroup -dynamic -tags=hw-class=IO io-pools psu create pgroup -dynamic -tags=hw-class=replica replica-pools psu create pgroup all-pools psu addto pgroup all-pools @io-pools @replica-pools ``` Modification: Update PGroup class to support poolgroups. Update addToPoolGroup/removeFromPoolGroup commands to support nested groups, identified by '@' prefix. Add unit tests to validate the behaviour. Result: More flexible pool manager config. Acked-by: Paul Millar Target: master Require-book: no Require-notes: yes --- .../src/main/markdown/config-PoolManager.md | 15 +++ .../java/diskCacheV111/poolManager/Link.java | 2 +- .../diskCacheV111/poolManager/PGroup.java | 71 +++++++++++- .../java/diskCacheV111/poolManager/Pool.java | 18 ++++ .../poolManager/PoolSelectionUnit.java | 3 + .../poolManager/PoolSelectionUnitV2.java | 63 +++++++---- .../poolManager/PoolSelectionUnitV2Test.java | 101 ++++++++++++++++++ 7 files changed, 250 insertions(+), 23 deletions(-) create mode 100644 modules/dcache/src/test/java/diskCacheV111/poolManager/PoolSelectionUnitV2Test.java diff --git a/docs/TheBook/src/main/markdown/config-PoolManager.md b/docs/TheBook/src/main/markdown/config-PoolManager.md index 036359fd920..a1ec3ef3adc 100644 --- a/docs/TheBook/src/main/markdown/config-PoolManager.md +++ b/docs/TheBook/src/main/markdown/config-PoolManager.md @@ -177,6 +177,21 @@ Will create pool group `zone-A-pools` and any existing pool as well as any new p > > Pools can't be manually added into dynamic groups with `psu addto pgroup` admin command. +#### Nested Pool Groups + +Pool groups can be grouped together into nested pool groups. The special symbol `@` used as prefix +to `psu addto pgroup` indicates, that the added object is a group: + + Example: + + psu create pgroup group-foo + psu create pgroup group-bar + psu create pgroup group-foo-bar + psu addto pgroup group-foo-bar @group-foo + psu addto pgroup group-foo-bar @group-bar + +The pool list of the nested group will be dynamically updated as soon as subgroup is updated. + #### Storage Classes The storage class is a string of the form `StoreName:StorageGroup@type-of-storage-system`, where `type-of-storage-system` denotes the type of storage system in use, and `StoreName`:`StorageGroup` is a string describing the storage class in a syntax which depends on the storage system. In general use `type-of-storage-system=osm`. diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/Link.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/Link.java index e7f6f63aad5..36b1c6197fb 100644 --- a/modules/dcache/src/main/java/diskCacheV111/poolManager/Link.java +++ b/modules/dcache/src/main/java/diskCacheV111/poolManager/Link.java @@ -83,7 +83,7 @@ public Collection getPools() { if (o instanceof Pool) { list.add((Pool) o); } else if (o instanceof PGroup) { - list.addAll(((PGroup) o)._poolList.values()); + list.addAll(((PGroup)o).getPools()); } } return list; diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/PGroup.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/PGroup.java index e7707e4871f..81ebe7a5fc3 100644 --- a/modules/dcache/src/main/java/diskCacheV111/poolManager/PGroup.java +++ b/modules/dcache/src/main/java/diskCacheV111/poolManager/PGroup.java @@ -1,13 +1,21 @@ package diskCacheV111.poolManager; +import com.google.common.graph.GraphBuilder; +import com.google.common.graph.Graphs; +import com.google.common.graph.MutableGraph; import diskCacheV111.poolManager.PoolSelectionUnit.SelectionPoolGroup; +import java.util.ArrayList; +import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; class PGroup extends PoolCore implements SelectionPoolGroup { private static final long serialVersionUID = 3883973457610397314L; final Map _poolList = new ConcurrentHashMap<>(); + final List _pgroupList = new CopyOnWriteArrayList<>(); private final boolean resilient; @@ -29,10 +37,67 @@ public boolean isPrimary() { @Override public String toString() { return getName() + "(links=" + _linkList.size() - + "; pools=" + _poolList.size() + "; resilient=" + resilient + ")"; + + "; pools=" + _poolList.size() + "; resilient=" + resilient + "; nested groups=" + _pgroupList + ")"; } - private String[] getPools() { - return _poolList.keySet().toArray(String[]::new); + @Override + public List getPools() { + List allPools = new ArrayList<>(_poolList.values()); + _pgroupList.forEach(g -> allPools.addAll(g.getPools())); + return allPools; + } + + + // check whatever there is a pool group that exist in there reference list + // IOW, A -> B -> C ; C -> A not allowed. + private void checkLoop(PGroup top, PGroup subGroup) { + var g = GraphBuilder + .directed() + .allowsSelfLoops(true) // we will check later + .build(); + + fillReferenceTree(top, subGroup, g); + + if (Graphs.hasCycle(g)) { + throw new IllegalArgumentException("Cyclic pool group reference not allowed: " + g.edges()); + } + } + + private void fillReferenceTree(PGroup top, PGroup subGroup, MutableGraph g) { + + if (!g.putEdge(top.getName(), subGroup.getName())) { + // the edge is not added, probably a duplicate + throw new IllegalArgumentException("Subgroup " + subGroup.getName() + " is already defined in " + top.getName()); + } + + for (PGroup ref : subGroup._pgroupList) { + if (!g.putEdge(subGroup.getName(), ref.getName())) { + // the edge is not added, probably a duplicate + throw new IllegalArgumentException("Subgroup " + ref.getName() + " is already defined in " + subGroup.getName()); + } + fillReferenceTree(subGroup, ref, g); + } + } + + public void addSubgroup(PGroup subGroup) { + checkLoop(this, subGroup); + _pgroupList.add(subGroup); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + PGroup group = (PGroup) o; + return getName().equals(group.getName()); + } + + @Override + public int hashCode() { + return Objects.hash(getName()); } } diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/Pool.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/Pool.java index 2ee6503ba22..22bc3be2470 100644 --- a/modules/dcache/src/main/java/diskCacheV111/poolManager/Pool.java +++ b/modules/dcache/src/main/java/diskCacheV111/poolManager/Pool.java @@ -10,6 +10,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -220,4 +221,21 @@ public void setCanonicalHostName(String hostName) { public Optional getCanonicalHostName() { return Optional.ofNullable(_hostName); } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Pool group = (Pool) o; + return getName().equals(group.getName()); + } + + @Override + public int hashCode() { + return Objects.hash(getName()); + } } diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnit.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnit.java index 62fcf9017af..bf0329d3ffe 100644 --- a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnit.java +++ b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnit.java @@ -4,6 +4,7 @@ import dmg.cells.nucleus.CellAddressCore; import java.net.UnknownHostException; import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Optional; @@ -193,6 +194,8 @@ interface SelectionPoolGroup extends SelectionEntity { boolean isPrimary(); boolean isResilient(); + + List getPools(); } interface SelectionLinkGroup extends SelectionEntity { diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java index 3e363fad842..d94894cda43 100644 --- a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java +++ b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java @@ -2040,11 +2040,6 @@ public void removeFromUnitGroup(String groupName, String unitName, boolean isNet public void removeFromPoolGroup(String groupName, String poolName) { wlock(); try { - Pool pool = _pools.get(poolName); - if (pool == null) { - throw new IllegalArgumentException("Pool not found : " - + poolName); - } PGroup group = _pGroups.get(groupName); if (group == null) { @@ -2052,13 +2047,32 @@ public void removeFromPoolGroup(String groupName, String poolName) { + groupName); } - if (group._poolList.get(poolName) == null) { - throw new IllegalArgumentException(poolName + " not member of " - + groupName); - } + if (poolName.charAt(0) == '@') { + PGroup nestedGroup = _pGroups.get(poolName.substring(1)); + if (nestedGroup == null) { + throw new IllegalArgumentException("Group not found : " + poolName); + } + + if (!group._pgroupList.remove(nestedGroup)) { + throw new IllegalArgumentException(nestedGroup + " not a subgroup of " + + groupName); + } + } else { - group._poolList.remove(poolName); - pool._pGroupList.remove(groupName); + Pool pool = _pools.get(poolName); + if (pool == null) { + throw new IllegalArgumentException("Pool not found : " + + poolName); + } + + if (group._poolList.get(poolName) == null) { + throw new IllegalArgumentException(poolName + " not member of " + + groupName); + } + + group._poolList.remove(poolName); + pool._pGroupList.remove(groupName); + } } finally { wunlock(); } @@ -2154,6 +2168,7 @@ public void removeLink(String name) { // // relations // + public void addToPoolGroup(String pGroupName, String poolName) { wlock(); @@ -2166,13 +2181,23 @@ public void addToPoolGroup(String pGroupName, String poolName) { throw new IllegalArgumentException( "Manual adding into dynamic pool is not allowed"); } - Pool pool = _pools.get(poolName); - if (pool == null) { - throw new IllegalArgumentException("Not found : " + poolName); - } - pool._pGroupList.put(group.getName(), group); - group._poolList.put(pool.getName(), pool); + if (poolName.charAt(0) == '@') { + var groupName = poolName.substring(1); + PGroup subGroup = _pGroups.get(groupName); + if (subGroup == null) { + throw new IllegalArgumentException("Subgroup not found : " + groupName); + } + group.addSubgroup(subGroup); + } else { + Pool pool = _pools.get(poolName); + if (pool == null) { + throw new IllegalArgumentException("Pool not found: : " + poolName); + } + + pool._pGroupList.put(group.getName(), group); + group._poolList.put(pool.getName(), pool); + } } finally { wunlock(); } @@ -2530,7 +2555,7 @@ protected void runlock() { return ""; } - public static final String hh_psu_addto_pgroup = " "; + public static final String hh_psu_addto_pgroup = " "; @AffectsSetup public String ac_psu_addto_pgroup_$_2(Args args) { @@ -2753,7 +2778,7 @@ public String ac_psu_ls_netunits(Args args) { return ""; } - public static final String hh_psu_removefrom_pgroup = " "; + public static final String hh_psu_removefrom_pgroup = " "; @AffectsSetup public String ac_psu_removefrom_pgroup_$_2(Args args) { diff --git a/modules/dcache/src/test/java/diskCacheV111/poolManager/PoolSelectionUnitV2Test.java b/modules/dcache/src/test/java/diskCacheV111/poolManager/PoolSelectionUnitV2Test.java new file mode 100644 index 00000000000..c50b55e54bc --- /dev/null +++ b/modules/dcache/src/test/java/diskCacheV111/poolManager/PoolSelectionUnitV2Test.java @@ -0,0 +1,101 @@ +package diskCacheV111.poolManager; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Before; +import org.junit.Test; + +public class PoolSelectionUnitV2Test { + + private PoolSelectionUnitV2 psu; + + @Before + public void setUp() { + psu = new PoolSelectionUnitV2(); + } + + @Test + public void testAddGroup() { + psu.createPoolGroup("group", false); + psu.getPoolGroups().containsKey("group"); + } + + @Test + public void testAddPoolToGroup() { + psu.createPoolGroup("group", false); + psu.createPool("poolA", true, false, false); + psu.addToPoolGroup("group", "poolA"); + + assertTrue(psu.getPoolGroups().get("group").getPools().contains(psu.getPool("poolA"))); + } + + @Test(expected = IllegalArgumentException.class) + public void testNestedCyclicGroup() { + psu.createPoolGroup("group", false); + psu.addToPoolGroup("group", "@group"); + } + + @Test(expected = IllegalArgumentException.class) + public void testNestedCyclicGroupChain() { + psu.createPoolGroup("groupA", false); + psu.createPoolGroup("groupB", false); + psu.createPoolGroup("groupC", false); + + psu.addToPoolGroup("groupA", "@groupB"); + psu.addToPoolGroup("groupB", "@groupC"); + psu.addToPoolGroup("groupC", "@groupA"); + } + + @Test + public void testNestedGroup() { + psu.createPoolGroup("group", false); + + psu.createPoolGroup("foo", false); + psu.createPool("pool-foo", true, false, false); + psu.addToPoolGroup("foo", "pool-foo"); + + psu.createPoolGroup("bar", false); + psu.createPool("pool-bar", true, false, false); + psu.addToPoolGroup("bar", "pool-bar"); + + psu.addToPoolGroup("group", "@foo"); + psu.addToPoolGroup("group", "@bar"); + + assertTrue(psu.getPoolGroups().get("group").getPools().contains(psu.getPool("pool-foo"))); + assertTrue(psu.getPoolGroups().get("group").getPools().contains(psu.getPool("pool-bar"))); + } + + @Test + public void testRemoveNestedGroup() { + psu.createPoolGroup("group", false); + psu.createPoolGroup("foo", false); + psu.createPoolGroup("bar", false); + + + psu.createPool("pool-foo", true, false, false); + psu.addToPoolGroup("foo", "pool-foo"); + + psu.createPool("pool-bar", true, false, false); + psu.addToPoolGroup("bar", "pool-bar"); + + psu.addToPoolGroup("group", "@foo"); + psu.addToPoolGroup("group", "@bar"); + + psu.removeFromPoolGroup("group", "@foo"); + + assertFalse(psu.getPoolGroups().get("group").getPools().contains(psu.getPool("pool-foo"))); + assertTrue(psu.getPoolGroups().get("group").getPools().contains(psu.getPool("pool-bar"))); + } + + @Test(expected = IllegalArgumentException.class) + public void testRemoveMissingNestedGroup() { + psu.createPoolGroup("group", false); + psu.createPoolGroup("foo", false); + psu.createPoolGroup("bar", false); + + psu.addToPoolGroup("group", "@foo"); + + psu.removeFromPoolGroup("group", "@bar"); + } +} \ No newline at end of file