Merge pull request #47432 from makortel/xrdAdaptorNoEmptyExcludeString

XrdAdaptor: Do not add empty exclude strings

Author: cmsbuild

Utilities/XrdAdaptor/src/XrdRequestManager.cc

@@ -255,7 +255,9 @@ void RequestManager::initialize(std::weak_ptr<RequestManager> self) {
       }
       if (!dataServer.empty()) {
         m_disabledSourceStrings.insert(dataServer);
-        m_disabledExcludeStrings.insert(excludeString);
+        if (not excludeString.empty()) {
+          m_disabledExcludeStrings.insert(excludeString);
+        }
       }
       // In this case, we didn't go anywhere - we stayed at the redirector and it gave us a file-not-found.
       if (lastUrl == new_filename) {
@@ -835,7 +837,9 @@ void RequestManager::requestFailure(std::shared_ptr<XrdAdaptor::ClientRequest> c
   // function may be called from within XrdCl::ResponseHandler::HandleResponseWithHosts
   // In such a case, if you close a file in the handler, it will deadlock
   m_disabledSourceStrings.insert(source_ptr->ID());
-  m_disabledExcludeStrings.insert(source_ptr->ExcludeID());
+  if (auto const &eid = source_ptr->ExcludeID(); not eid.empty()) {
+    m_disabledExcludeStrings.insert(eid);
+  }
   m_disabledSources.insert(source_ptr);
 
   std::unique_lock<std::recursive_mutex> sentry(m_source_mutex);

Utilities/XrdAdaptor/src/XrdSource.cc

@@ -293,6 +293,23 @@ void Source::determineHostExcludeString(XrdCl::File &file, const XrdCl::HostList
   // We assume this is a federation context if there's at least a regional, dCache door,
   // and dCache pool server (so, more than 2 servers!).
 
+  // Further explanation of the motivation from recollections of Brian Bockelman
+  // 1. The way dCache sites are (were?) integrated into AAA they
+  //    required a standalone server, separate from dCache itself,
+  //    that would advertise file availability (via running the cmsd
+  //    component).
+  // 2. When a file-open failed for a dCache "pool" (the disk server
+  //    itself), adding the pool name to the exclude list was useless
+  //    because the client was subsequently redirected to server in
+  //    item (1) which, unlike native XRootD solutions, was not aware
+  //    of what pool the client would land on (i.e., it didn't know it
+  //    would redirect again to the same bad pool).
+  // 3. So, the code had to take an informed guess as to which the
+  //    site integration server (item 1) was by walking up the list of
+  //    redirects and try to hit the first native XRootD source.
+  //    That's what the heuristic in
+  //    Source::determineHostExcludeString is trying to do.
+
   exclude = "";
   if (hostList && (hostList->size() > 3) && isDCachePool(file, hostList)) {
     const XrdCl::HostInfo &info = (*hostList)[hostList->size() - 3];