From dcf982e91250998bcd5ed97ef40adef6552a4ae8 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 19 Feb 2026 18:25:53 +0000 Subject: [PATCH 1/2] poolManager: handle null criteria in match using wildcard units The non-test changes introduced in 96c4db4f8b broke hot file replication because they replaced hardcoded protocol and network values with null. In PoolSelectionUnitV2.match, null criteria resulted in those dimensions not being represented in the units list, causing links with protocol or network unit group constraints to fail to match (due to the fitCount check and the requirement that all unit groups of a link must be satisfied). This patch introduces a wildcard unit mechanism. When protocol or network criteria are null, a virtual unit is added that belongs to all unit groups of that type. This allows links with such constraints to match while still ensuring that other dimensions (like storage units) must be satisfied. Wildcard units are cached and invalidated whenever the selection setup changes to ensure performance in hot code paths. A regression test has been added to PoolSelectionUnitV2Test to verify that null criteria correctly match constrained links. Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> --- .../poolManager/PoolSelectionUnitV2.java | 28 +++++++++++++++++++ .../poolManager/PoolSelectionUnitV2Test.java | 13 +++++++++ 2 files changed, 41 insertions(+) diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java index 598a927d39c..56462c723b3 100644 --- a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java +++ b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java @@ -104,6 +104,9 @@ public String getVersion() { private boolean _allPoolsActive; public boolean _cachingEnabeled; + private transient Unit _wildcardProtocolUnit; + private transient Unit _wildcardNetUnit; + /** * Ok, this is the critical part of PoolManager, but (!!!) the whole select path is READ-ONLY, * unless we change setup. So ReadWriteLock is what we are looking for, while is a point of @@ -720,6 +723,15 @@ public PoolPreferenceLevel[] match(DirectionType type, String netUnitName, return result; } + private Unit wildcardUnit(UnitType type) { + Unit wildcard = new Unit("wildcard", type); + _units.values().stream() + .filter(u -> u.getType() == type) + .flatMap(u -> u._uGroupList.values().stream()) + .forEach(ug -> wildcard._uGroupList.put(ug.getName(), ug)); + return wildcard; + } + private void resolveStorageUnit(List list, String storeUnitName) { if (_useRegex) { Unit universalCoverage = null; @@ -790,6 +802,13 @@ private void addProtocolUnit(List list, String protocolUnitName) { LOGGER.debug("matching protocol unit found: {}", unit); list.add(unit); + } else { + Unit wildcard = _wildcardProtocolUnit; + if (wildcard == null) { + wildcard = wildcardUnit(PROTOCOL); + _wildcardProtocolUnit = wildcard; + } + list.add(wildcard); } } @@ -831,6 +850,13 @@ private void addNetUnit(List list, String netUnitName) { throw new IllegalArgumentException( "NetUnit not resolved : " + netUnitName); } + } else { + Unit wildcard = _wildcardNetUnit; + if (wildcard == null) { + wildcard = wildcardUnit(NET); + _wildcardNetUnit = wildcard; + } + list.add(wildcard); } } @@ -2662,6 +2688,8 @@ private String poolCountDescriptionFor(int count) { protected void wlock() { _psuWriteLock.lock(); + _wildcardProtocolUnit = null; + _wildcardNetUnit = null; if (_cachingEnabeled) { cachedMatchValue.invalidateAll(); } diff --git a/modules/dcache/src/test/java/diskCacheV111/poolManager/PoolSelectionUnitV2Test.java b/modules/dcache/src/test/java/diskCacheV111/poolManager/PoolSelectionUnitV2Test.java index 48a6c009d91..1bae3729684 100644 --- a/modules/dcache/src/test/java/diskCacheV111/poolManager/PoolSelectionUnitV2Test.java +++ b/modules/dcache/src/test/java/diskCacheV111/poolManager/PoolSelectionUnitV2Test.java @@ -443,4 +443,17 @@ private void whenMatchIsCalledWith(String params) { assertNull("Unexpected exception", e); } } + + /** + * Regression test for hot file replication issue introduced in 96c4db4f8b. + * When protocol and network are null, they should match any protocol or network requirement + * of a link. + */ + @Test + public void testThatReadWithNullProtocolAndNetMatchesTapePools() { + diskCacheV111.vehicles.StorageInfo storageInfo = diskCacheV111.vehicles.GenericStorageInfo.valueOf("tape.dcache-devel-test@enstore", "*"); + org.dcache.vehicles.FileAttributes fileAttributes = org.dcache.vehicles.FileAttributes.ofStorageInfo(storageInfo); + levels = psu.match(PoolSelectionUnit.DirectionType.READ, null, null, fileAttributes, null, p -> false); + assertThatPoolsAre(TAPE_POOLS); + } } \ No newline at end of file From 6466a4e3e87cd36b4cdf75586fd79c01b055304b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 19 Feb 2026 19:35:15 +0000 Subject: [PATCH 2/2] Fix regression in pool selection for hot file replication Commit 96c4db4f8b introduced a stricter link matching logic that failed when protocol or network criteria were null. Internal dCache operations like hot file replication often omit these criteria. This patch allows PoolSelectionUnitV2 to match links with protocol or network constraints even if the request does not provide these criteria. The matching engine now correctly identifies and satisfies these dimensions by default if they are missing from the request. Includes a regression test in PoolSelectionUnitV2Test. Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com> --- .../diskCacheV111/poolManager/LinkMap.java | 14 ++--- .../poolManager/PoolSelectionUnitV2.java | 53 +++++++------------ 2 files changed, 26 insertions(+), 41 deletions(-) diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/LinkMap.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/LinkMap.java index 668b5ca6b37..3c6ecb47c2e 100644 --- a/modules/dcache/src/main/java/diskCacheV111/poolManager/LinkMap.java +++ b/modules/dcache/src/main/java/diskCacheV111/poolManager/LinkMap.java @@ -1,19 +1,18 @@ package diskCacheV111.poolManager; -import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.Iterator; -import java.util.List; import java.util.Map; class LinkMap { - private static class LinkMapEntry { + static class LinkMapEntry { - private final Link _link; - private int _counter; + final Link _link; + int _counter; - private LinkMapEntry(Link link) { + LinkMapEntry(Link link) { _link = link; _counter = link._uGroupList.size() - 1; } @@ -37,4 +36,7 @@ void addLink(Link link) { } } + Collection entries() { + return _linkHash.values(); + } } diff --git a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java index 56462c723b3..0d89afcbaa6 100644 --- a/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java +++ b/modules/dcache/src/main/java/diskCacheV111/poolManager/PoolSelectionUnitV2.java @@ -104,9 +104,6 @@ public String getVersion() { private boolean _allPoolsActive; public boolean _cachingEnabeled; - private transient Unit _wildcardProtocolUnit; - private transient Unit _wildcardNetUnit; - /** * Ok, this is the critical part of PoolManager, but (!!!) the whole select path is READ-ONLY, * unless we change setup. So ReadWriteLock is what we are looking for, while is a point of @@ -706,7 +703,8 @@ public PoolPreferenceLevel[] match(DirectionType type, String netUnitName, addNetUnit(units, netUnitName); LinkGroup linkGroup = resolveLinkGroup(linkGroupName); - Set sortedSet = findMatchingLinks(units, linkGroup, type); + Set sortedSet = findMatchingLinks(units, linkGroup, type, + protocolUnitName == null, netUnitName == null); List> linkLists = matchPreferences(type, sortedSet); result = buildPreferenceLevels(type, linkLists, fileAttributes, exclude); @@ -723,15 +721,6 @@ public PoolPreferenceLevel[] match(DirectionType type, String netUnitName, return result; } - private Unit wildcardUnit(UnitType type) { - Unit wildcard = new Unit("wildcard", type); - _units.values().stream() - .filter(u -> u.getType() == type) - .flatMap(u -> u._uGroupList.values().stream()) - .forEach(ug -> wildcard._uGroupList.put(ug.getName(), ug)); - return wildcard; - } - private void resolveStorageUnit(List list, String storeUnitName) { if (_useRegex) { Unit universalCoverage = null; @@ -802,13 +791,6 @@ private void addProtocolUnit(List list, String protocolUnitName) { LOGGER.debug("matching protocol unit found: {}", unit); list.add(unit); - } else { - Unit wildcard = _wildcardProtocolUnit; - if (wildcard == null) { - wildcard = wildcardUnit(PROTOCOL); - _wildcardProtocolUnit = wildcard; - } - list.add(wildcard); } } @@ -850,13 +832,6 @@ private void addNetUnit(List list, String netUnitName) { throw new IllegalArgumentException( "NetUnit not resolved : " + netUnitName); } - } else { - Unit wildcard = _wildcardNetUnit; - if (wildcard == null) { - wildcard = wildcardUnit(NET); - _wildcardNetUnit = wildcard; - } - list.add(wildcard); } } @@ -874,18 +849,28 @@ private LinkGroup resolveLinkGroup(String linkGroupName) { } private Set findMatchingLinks(List units, LinkGroup linkGroup, - DirectionType type) { + DirectionType type, boolean ignoreProtocol, boolean ignoreNet) { Set sortedSet = new TreeSet<>(new LinkComparator(type)); LinkMap matchingLinks = new LinkMap(); - int fitCount = units.size(); for (Unit unit : units) { matchingLinks = match(matchingLinks, unit, linkGroup, type); } - Iterator linkIterator = matchingLinks.iterator(); - while (linkIterator.hasNext()) { - Link link = linkIterator.next(); - if (link._uGroupList.size() <= fitCount) { + for (LinkMap.LinkMapEntry entry : matchingLinks.entries()) { + Link link = entry._link; + int ignored = 0; + if (ignoreProtocol || ignoreNet) { + for (UGroup uGroup : link._uGroupList.values()) { + UnitType groupType = uGroup._unitList.values().stream() + .findFirst().map(Unit::getType).orElse(null); + if ((ignoreProtocol && groupType == PROTOCOL) + || (ignoreNet && groupType == NET)) { + entry._counter--; + ignored++; + } + } + } + if (entry._counter <= 0 && link._uGroupList.size() <= units.size() + ignored) { sortedSet.add(link); } } @@ -2688,8 +2673,6 @@ private String poolCountDescriptionFor(int count) { protected void wlock() { _psuWriteLock.lock(); - _wildcardProtocolUnit = null; - _wildcardNetUnit = null; if (_cachingEnabeled) { cachedMatchValue.invalidateAll(); }