core: make reader engine selection collective and fail cleanly

eisenhauer · eisenhauer · commit acd9c0aa2324 · 2026-06-22T13:55:25.000-04:00
It probed the filesystem from every rank and, for a non-existent path, could pick BP3 and throw only on rank 0, hanging the others (#5098). Now the probe runs on rank 0 and the chosen engine (or a clear error) is broadcast, so a missing location fails collectively with a SetEngine hint instead of hanging.
diff --git a/docs/user_guide/source/introduction/whatsnew.rst b/docs/user_guide/source/introduction/whatsnew.rst
@@ -26,6 +26,16 @@ only the **BP5** reader supports them, and only when opened in
 ``Mode::ReadRandomAccess`` with a reentrant file transport (POSIX). See the
 Selection chapter for full details.
 
+Collective reader engine selection
+----------------------------------
+
+When a reader does not set an engine explicitly, the engine type is
+auto-detected from the filesystem. That probing now happens on rank 0 and is
+broadcast, instead of every rank issuing its own ``stat()`` calls. If nothing
+exists at the given location, ``IO::Open`` now fails collectively with a clear
+error (and a hint to call ``SetEngine`` for a stream that has not been created
+yet) rather than selecting BP3 and hanging the non-root MPI ranks.
+
 SST Mercury Data Plane
 -----------------------
 
diff --git a/source/adios2/core/IO.cpp b/source/adios2/core/IO.cpp
@@ -12,7 +12,9 @@
 #include <memory>
 #include <mutex>
 #include <sstream>
+#include <stdexcept>
 #include <utility> // std::pair
+#include <vector>
 
 #include "adios2/common/ADIOSMacros.h"
 
@@ -592,14 +594,27 @@ Engine &IO::Open(const std::string &name, const Mode mode, helper::Comm comm, co
         std::transform(engineTypeLC.begin(), engineTypeLC.end(), engineTypeLC.begin(), ::tolower);
     }
 
-    /* Second step in handling virtual engines */
-    /* BPFile for read needs to use BP5, BP4, or BP3 depending on the file's
-     * version
-     */
-    if ((engineTypeLC == "file" || engineTypeLC == "bpfile" || engineTypeLC == "bp" ||
-         isDefaultEngine))
+    // Engine selection probes the filesystem on rank 0 only and broadcasts the
+    // chosen engine type, so a missing target fails on every rank instead of a
+    // lone rank-0 throw that hangs the others.
+    const bool isVirtualFile = (engineTypeLC == "file" || engineTypeLC == "bpfile" ||
+                                engineTypeLC == "bp" || engineTypeLC == "filestream");
+
+    if (isDefaultEngine || isVirtualFile)
     {
-        if (helper::EndsWith(name, ".h5", false))
+        if (engineTypeLC == "filestream")
+        {
+            // FileStream streams BP4/BP5 file output only (not DAOS, timeseries,
+            // etc.). A not-yet-created stream is normal, so BPVersionLocal returns
+            // '4' for a missing path and the engine waits rather than erroring.
+            char version = '4';
+            if (comm.Rank() == 0)
+            {
+                version = helper::BPVersionLocal(name);
+            }
+            engineTypeLC = std::string("bp") + comm.BroadcastValue(version);
+        }
+        else if (helper::EndsWith(name, ".h5", false))
         {
             engineTypeLC = "hdf5";
         }
@@ -613,49 +628,66 @@ Engine &IO::Open(const std::string &name, const Mode mode, helper::Comm comm, co
         }
         else if ((mode_to_use == Mode::Read) || (mode_to_use == Mode::ReadRandomAccess))
         {
-            // Check if TarInfo parameter indicates a TAR file
             auto it = m_Parameters.find("TarInfo");
             if (it != m_Parameters.end())
             {
+                // TarInfo is a parameter (same on every rank), so guess directly.
                 engineTypeLC = lf_GuessEngineFromTAR(it->second);
             }
-            else if (helper::IsDAOSDataset(name))
-            {
-                // Must precede the generic directory branch: a DAOS
-                // dataset is also a directory and contains an mmd.0,
-                // so BPVersion would otherwise mis-detect it as BP5.
-                engineTypeLC = "daos";
-            }
-            else if (adios2sys::SystemTools::FileIsDirectory(name))
-            {
-                char v = helper::BPVersion(name, comm, m_TransportsParameters);
-                engineTypeLC = "bp";
-                engineTypeLC.push_back(v);
-            }
-            else if (adios2sys::SystemTools::FileIsDirectory(name + ".tier0"))
-            {
-                engineTypeLC = "mhs";
-            }
             else
             {
-                if (helper::EndsWith(name, ".bp", false))
-                {
-                    engineTypeLC = "bp3";
-                }
-                else
+                // Rank 0 picks the engine from the on-disk dataset; an empty
+                // result means nothing exists at the location.
+                std::vector<char> selected;
+                if (comm.Rank() == 0)
                 {
-                    /* We need to figure out the type of file
-                     * from the file itself
-                     */
-                    if (helper::IsHDF5File(name, *this, comm, m_TransportsParameters))
+                    std::string sel;
+                    if (adios2sys::SystemTools::PathExists(name) ||
+                        adios2sys::SystemTools::PathExists(name + ".tier0"))
                     {
-                        engineTypeLC = "hdf5";
+                        // DAOS must precede the directory branch: a DAOS dataset is
+                        // also a directory containing an mmd.0 and would mis-detect
+                        // as BP5.
+                        if (helper::IsDAOSDataset(name))
+                        {
+                            sel = "daos";
+                        }
+                        else if (adios2sys::SystemTools::FileIsDirectory(name))
+                        {
+                            sel = std::string("bp") + helper::BPVersionLocal(name);
+                        }
+                        else if (adios2sys::SystemTools::FileIsDirectory(name + ".tier0"))
+                        {
+                            sel = "mhs";
+                        }
+                        else if (helper::EndsWith(name, ".bp", false))
+                        {
+                            sel = "bp3";
+                        }
+                        else
+                        {
+                            sel = helper::IsHDF5FileLocal(name, *this, comm, m_TransportsParameters)
+                                      ? "hdf5"
+                                      : "bp3";
+                        }
                     }
-                    else
+                    selected.assign(sel.begin(), sel.end());
+                }
+                comm.BroadcastVector(selected);
+                if (selected.empty())
+                {
+                    // Built identically on every rank from 'name', so all ranks
+                    // throw together without broadcasting the message.
+                    std::string err = "Cannot determine the engine for reading \"" + name +
+                                      "\": nothing exists at that location.";
+                    if (mode_to_use == Mode::Read)
                     {
-                        engineTypeLC = "bp3";
+                        err += "  If it is a stream that has not been created yet, select "
+                               "the engine explicitly with IO::SetEngine().";
                     }
+                    helper::Throw<std::runtime_error>("Core", "IO", "Open", err);
                 }
+                engineTypeLC.assign(selected.begin(), selected.end());
             }
         }
         else
@@ -665,29 +697,6 @@ Engine &IO::Open(const std::string &name, const Mode mode, helper::Comm comm, co
         }
     }
 
-    /* Note: Mismatch between BP4/BP5 writer and FileStream reader is not
-       handled if writer has not created the directory yet, when FileStream
-       falls back to default (BP4) */
-    if (engineTypeLC == "filestream")
-    {
-        if (helper::EndsWith(name, ".ats", false))
-        {
-            engineTypeLC = "timeseries";
-        }
-        else if (helper::IsDAOSDataset(name))
-        {
-            engineTypeLC = "daos";
-        }
-        else
-        {
-            char v = helper::BPVersion(name, comm, m_TransportsParameters);
-            engineTypeLC = "bp";
-            engineTypeLC.push_back(v);
-        }
-        // std::cout << "Engine " << engineTypeLC << " selected for FileStream"
-        //          << std::endl;
-    }
-
     // For the inline engine, there must be exactly 1 reader, and exactly 1
     // writer.
     if (engineTypeLC == "inline")
diff --git a/source/adios2/helper/adiosSystem.cpp b/source/adios2/helper/adiosSystem.cpp
@@ -124,62 +124,55 @@ int ExceptionToError(const std::string &function)
     }
 }
 
-bool IsHDF5File(const std::string &name, core::IO &io, helper::Comm &comm,
-                const std::vector<Params> &transportsParameters) noexcept
+bool IsHDF5FileLocal(const std::string &name, core::IO &io, helper::Comm &comm,
+                     const std::vector<Params> &transportsParameters) noexcept
 {
     bool isHDF5 = false;
-    if (!comm.Rank())
+    try
     {
-        try
+        transportman::TransportMan tm(io, comm);
+        if (transportsParameters.empty())
         {
-            transportman::TransportMan tm(io, comm);
-            if (transportsParameters.empty())
-            {
-                std::vector<Params> defaultTransportParameters(1);
-                defaultTransportParameters[0]["transport"] = "File";
-                tm.OpenFiles({name}, adios2::Mode::Read, defaultTransportParameters, false);
-            }
-            else
-            {
-                tm.OpenFiles({name}, adios2::Mode::Read, transportsParameters, false);
-            }
-            const unsigned char HDF5Header[8] = {137, 72, 68, 70, 13, 10, 26, 10};
-            if (tm.GetFileSize(0) >= 8)
-            {
-                char header[8];
-                tm.ReadFile(header, 8, 0);
-                tm.CloseFiles();
-                isHDF5 = !std::memcmp(header, HDF5Header, 8);
-            }
+            std::vector<Params> defaultTransportParameters(1);
+            defaultTransportParameters[0]["transport"] = "File";
+            tm.OpenFiles({name}, adios2::Mode::Read, defaultTransportParameters, false);
         }
-        catch (std::ios_base::failure &)
+        else
         {
-            isHDF5 = false;
+            tm.OpenFiles({name}, adios2::Mode::Read, transportsParameters, false);
+        }
+        const unsigned char HDF5Header[8] = {137, 72, 68, 70, 13, 10, 26, 10};
+        if (tm.GetFileSize(0) >= 8)
+        {
+            char header[8];
+            tm.ReadFile(header, 8, 0);
+            tm.CloseFiles();
+            isHDF5 = !std::memcmp(header, HDF5Header, 8);
         }
     }
-    size_t flag = (isHDF5 ? 1 : 0);
-    flag = comm.BroadcastValue(flag);
-    return (flag == 1);
+    catch (std::ios_base::failure &)
+    {
+        isHDF5 = false;
+    }
+    return isHDF5;
 }
 
-char BPVersion(const std::string &name, helper::Comm &comm,
-               const std::vector<Params> &transportsParameters) noexcept
+bool IsHDF5File(const std::string &name, core::IO &io, helper::Comm &comm,
+                const std::vector<Params> &transportsParameters) noexcept
 {
-    char version = '4'; // default result
+    size_t flag = 0;
     if (!comm.Rank())
     {
-        std::string mmdFileName = name + PathSeparator + "mmd.0";
-        if (adios2sys::SystemTools::PathExists(mmdFileName))
-        {
-            version = '5';
-        }
-        else
-        {
-            version = '4';
-        }
+        flag = IsHDF5FileLocal(name, io, comm, transportsParameters) ? 1 : 0;
     }
-    version = comm.BroadcastValue(version);
-    return version;
+    flag = comm.BroadcastValue(flag);
+    return (flag == 1);
+}
+
+char BPVersionLocal(const std::string &name) noexcept
+{
+    const std::string mmdFileName = name + PathSeparator + "mmd.0";
+    return adios2sys::SystemTools::PathExists(mmdFileName) ? '5' : '4';
 }
 
 bool IsDAOSDataset(const std::string &name) noexcept
diff --git a/source/adios2/helper/adiosSystem.h b/source/adios2/helper/adiosSystem.h
@@ -73,8 +73,11 @@ int ExceptionToError(const std::string &function);
 
 bool IsHDF5File(const std::string &name, core::IO &io, helper::Comm &comm,
                 const std::vector<Params> &transportsParameters) noexcept;
-char BPVersion(const std::string &name, helper::Comm &comm,
-               const std::vector<Params> &transportsParameters) noexcept;
+/** Rank-local IsHDF5File: no internal broadcast; call from a single rank. */
+bool IsHDF5FileLocal(const std::string &name, core::IO &io, helper::Comm &comm,
+                     const std::vector<Params> &transportsParameters) noexcept;
+/** Rank-local BP version sniff (no broadcast): '5' if an mmd.0 is present, else '4'. */
+char BPVersionLocal(const std::string &name) noexcept;
 
 /** Return true if 'name' is a dataset directory written by the DAOS
  *  engine.  Identified by the data_oids.txt index file the DAOS engine
diff --git a/testing/adios2/interface/TestADIOSInterface.cpp b/testing/adios2/interface/TestADIOSInterface.cpp
@@ -169,6 +169,18 @@ TEST_F(ADIOS2_CXX_API_IO, EngineDefault)
     engine.Close();
 }
 
+// Auto-selection (no SetEngine) on a missing path must throw on every rank, not
+// just rank 0, which otherwise hung the others (issue #5098).
+TEST_F(ADIOS2_CXX_API_IO, EngineSelectionMissingFileThrows)
+{
+    EXPECT_THROW(m_Io.Open("ADIOSEngineSelectionMissingFile.bp", adios2::Mode::Read),
+                 std::exception);
+#if ADIOS2_USE_MPI
+    // Returns only if every rank threw rather than hanging in Open.
+    MPI_Barrier(MPI_COMM_WORLD);
+#endif
+}
+
 template <class T>
 struct MyData
 {
