dcache-xroot: (door) ignore tried if property false instead of return…

…ing error

Motivation:

https://rb.dcache.org/r/11955/
master@261e78639548adadfaea38827c47efe92e1a2f41

https://rb.dcache.org/r/12387/
2231b12

introduced support for the 'tried' CGI, and
a property to handle it as an option.

Currently in the case where there are tried
indicators but the option is off, we fail
the transaction so the client knows immediately
it will not work.

However, this may create unnecessary failures
in some scenarios where it may not be essential for
dCache to support this option.

On the other hand, allowing the client to believe
it is supported may end up in the client repeatedly
retrying and then timing out.

Of the two possibilities, we now believe the second
less problematic than the first.

See also: #5811

Modification:

Do not return an error if there is a tried CGI
and xrootd.enable.tried-hosts is not true, but
simply ignore the attribute when communicating
with the Pool Manager.

Also ignore it if triedrc is not defined,
or if tiredrc is not enoent or ioerr.

Also changed the default value of the property
back to true.

Result:

Hopefully dCache will be less cranky in more
common situations.

Target: master
Request: 7.0
Request: 6.2
Request: 6.1
Patch: https://rb.dcache.org/r/12938/
Acked-by: Paul
Acked-by: Dmitry

docs/TheBook/src/main/markdown/config-xrootd.md

@@ -258,6 +258,29 @@ The same argument holds for many strong authentication mechanisms - for example,
 
 To allow local site’s administrators to override remote security settings, write access can be further restricted to few directories (based on the local namespace, the `pnfs`). Setting `xrootd access to read-only has the highest priority, overriding all other settings.
 
+### Tried-hosts
+
+Xrootd uses the path URL CGI "tried" and "triedrc" as hints to the
+redirector/manager not to reselect a data source because of some
+error condition or preference.  dCache provides limited support for
+this attribute.  In particular, it will honor it in the case that
+the indicated cause suggests some error previously encountered
+that suggests an IO malfunction on the node.
+
+The property
+
+```
+xrootd.enable.tried-hosts
+```
+
+is true by default. When it is off, the 'tried' element on the path
+is simply ignored.  dCache also ignored the tried hosts when 'triedrc'
+is not provided, or when it is not 'enoent' or 'ioerr'.  In the latter
+two cases, the xrootd door will forward the list of previously tried
+hosts to the Pool Manager to ask that they be excluded from selection.
+
+See ``xrootd.properties`` for further information.
+
 ### Other configuration options
 
 The `xrootd-door` has several other configuration properties. You can

modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdRedirectHandler.java

@@ -92,6 +92,7 @@
 import org.dcache.xrootd.util.OpaqueStringParser;
 import org.dcache.xrootd.util.ParseException;
 
+import static java.util.stream.Collectors.toSet;
 import static org.dcache.xrootd.CacheExceptionMapper.xrootdErrorCode;
 import static org.dcache.xrootd.CacheExceptionMapper.xrootdException;
 import static org.dcache.xrootd.protocol.XrootdProtocol.*;
@@ -104,8 +105,50 @@ public class XrootdRedirectHandler extends ConcurrentXrootdRequestHandler
     private static final Logger _log =
         LoggerFactory.getLogger(XrootdRedirectHandler.class);
 
-    private final XrootdDoor _door;
+    /*
+     *  REVISIT
+     *
+     *  This enum has been placed temporarily in the door in order to expedite
+     *  patching the behavior of dCache with respect to the 'tried' path GGI (tried-hosts).
+     *
+     *  A subsequent patch will move this to the xrootd4j library where it belongs, and
+     *  eliminate this definition when the library dependency is updated.
+     */
+    enum TriedRc {
+        ENOENT("enoent", "The file was not found at the listed hosts."),
+        IOERR("ioerr", "The client received an I/O error on the listed hosts."),
+        FSERR("fserr", "The client received a non-I/O error from the file system."),
+        SRVERR("srverr", "The client received a server-related error."),
+        RESEL("resel", "The client is trying to find a better server."),
+        RESEG("reseg", "The client is globally trying to find a better server.");
+
+        private static final Set<String> KEYS = EnumSet.allOf(TriedRc.class)
+                                                       .stream()
+                                                       .map(TriedRc::key)
+                                                       .collect(toSet());
+        private final String key;
+        private final String description;
+
+        TriedRc(String key, String description) {
+            this.key = key;
+            this.description = description;
+        }
+
+        public String key() {
+            return key;
+        }
+
+        public String description() {
+            return description;
+        }
+
+        static Set<String> keys()
+        {
+            return KEYS;
+        }
+    }
 
+    private final XrootdDoor _door;
     private Restriction _authz = Restrictions.denyAll();
     private OptionalLong _maximumUploadSize = OptionalLong.empty();
     private final Map<String, String> _appIoQueues;
@@ -560,20 +603,34 @@ protected XrootdResponse<OpenRequest> doOnOpen(ChannelHandlerContext ctx, OpenRe
     }
 
     private Set<String> extractTriedHosts(Map<String, String> opaque)
-        throws XrootdException
     {
         String tried = Strings.emptyToNull(opaque.get("tried"));
-        if (tried == null) {
+        String rc = Strings.emptyToNull(opaque.get("triedrc"));
+
+        if (tried == null || rc == null) {
+            _log.debug("tried {}, triedrc {}, ignoring.", tried, rc);
             return Collections.EMPTY_SET;
-        } else if (!_door.isTriedHostsEnabled()) {
-            throw new XrootdException(kXR_InvalidRequest,
-                                      "tried hosts option not supported.");
+        }
+
+        if (!_door.isTriedHostsEnabled()) {
+            _log.debug("tried hosts option not enabled, ignoring 'tried={}'.", tried);
+            return Collections.EMPTY_SET;
+        }
+
+        TriedRc triedRc = TriedRc.valueOf(rc.toUpperCase());
+        _log.debug("tried {}, triedrc {}, cause {}.", tried, triedRc.key(), triedRc.description());
+
+        switch (triedRc) {
+            case ENOENT:
+            case IOERR:
+                break;
+            default:
+                return Collections.EMPTY_SET;
         }
 
         Set<String> triedHosts
-                        = Arrays.stream(tried.split(","))
-                                .collect(Collectors.toSet());
-        _log.info("TRIED : {}", triedHosts);
+            = Arrays.stream(tried.split(",")).collect(Collectors.toSet());
+        _log.debug("tried hosts : {}", triedHosts);
 
         return triedHosts;
     }

skel/share/defaults/xrootd.properties

@@ -253,20 +253,22 @@ xrootd.query-config!role = none
 #        manager-role server that it wishes to exclude data servers on the
 #        given host because a previous attempt to use it failed.
 #
-#        Turning this on in dCache is optional.  When it is not enabled,
-#        and the xrootd client sends the option, the door will respond with
-#        an unsupported operation error.
+#        The list of hosts is ignored by dCache pool selection in the following cases:
+#
+#        1. when it is not enabled
+#        2. when the client has not provided a 'triedrc' (error code giving the reason)
+#        3. when the error code is other than 'enoent' (file not found) or 'ioerr'
 #
 #        It is recommended that if this option is activated, you make sure
 #        that all doors and pools that could be used as a third-party copy
 #        source support TLS (strict or optional) if the client is to be
 #        used with the 'xroots' protocol.  This is because the vanilla xrootd
 #        third-party copy will send a retried=<host> back to the door if
 #        it expects to use TLS and TLS fails.  When this happen, if the file
-#        has replicas only on that host, access to the file will be suspended
-#        to all clients by the PoolManager.
+#        has replicas only on that host, the client will retry uselessly until it
+#        times out.  Otherwise, setting this to false would be advisable.
 #
-(one-of?true|false)xrootd.enable.tried-hosts=false
+(one-of?true|false)xrootd.enable.tried-hosts=true
 
 #  Signed hash verification ----- see dcache.properties
 #