Skip to content

Commit 5758a59

Browse files
committed
core: make reader engine selection collective and fail cleanly
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. (cherry picked from commit acd9c0a) Backport adaptation: dropped the DAOS auto-detect branch (the DAOS engine and helper::IsDAOSDataset are not in release_212) and the master-only whatsnew section.
1 parent 915a62f commit 5758a59

4 files changed

Lines changed: 114 additions & 93 deletions

File tree

source/adios2/core/IO.cpp

Lines changed: 62 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
#include <memory>
1313
#include <mutex>
1414
#include <sstream>
15+
#include <stdexcept>
1516
#include <utility> // std::pair
17+
#include <vector>
1618

1719
#include "adios2/common/ADIOSMacros.h"
1820

@@ -592,14 +594,27 @@ Engine &IO::Open(const std::string &name, const Mode mode, helper::Comm comm, co
592594
std::transform(engineTypeLC.begin(), engineTypeLC.end(), engineTypeLC.begin(), ::tolower);
593595
}
594596

595-
/* Second step in handling virtual engines */
596-
/* BPFile for read needs to use BP5, BP4, or BP3 depending on the file's
597-
* version
598-
*/
599-
if ((engineTypeLC == "file" || engineTypeLC == "bpfile" || engineTypeLC == "bp" ||
600-
isDefaultEngine))
597+
// Engine selection probes the filesystem on rank 0 only and broadcasts the
598+
// chosen engine type, so a missing target fails on every rank instead of a
599+
// lone rank-0 throw that hangs the others.
600+
const bool isVirtualFile = (engineTypeLC == "file" || engineTypeLC == "bpfile" ||
601+
engineTypeLC == "bp" || engineTypeLC == "filestream");
602+
603+
if (isDefaultEngine || isVirtualFile)
601604
{
602-
if (helper::EndsWith(name, ".h5", false))
605+
if (engineTypeLC == "filestream")
606+
{
607+
// FileStream streams BP4/BP5 file output only (not DAOS, timeseries,
608+
// etc.). A not-yet-created stream is normal, so BPVersionLocal returns
609+
// '4' for a missing path and the engine waits rather than erroring.
610+
char version = '4';
611+
if (comm.Rank() == 0)
612+
{
613+
version = helper::BPVersionLocal(name);
614+
}
615+
engineTypeLC = std::string("bp") + comm.BroadcastValue(version);
616+
}
617+
else if (helper::EndsWith(name, ".h5", false))
603618
{
604619
engineTypeLC = "hdf5";
605620
}
@@ -613,42 +628,59 @@ Engine &IO::Open(const std::string &name, const Mode mode, helper::Comm comm, co
613628
}
614629
else if ((mode_to_use == Mode::Read) || (mode_to_use == Mode::ReadRandomAccess))
615630
{
616-
// Check if TarInfo parameter indicates a TAR file
617631
auto it = m_Parameters.find("TarInfo");
618632
if (it != m_Parameters.end())
619633
{
634+
// TarInfo is a parameter (same on every rank), so guess directly.
620635
engineTypeLC = lf_GuessEngineFromTAR(it->second);
621636
}
622-
else if (adios2sys::SystemTools::FileIsDirectory(name))
623-
{
624-
char v = helper::BPVersion(name, comm, m_TransportsParameters);
625-
engineTypeLC = "bp";
626-
engineTypeLC.push_back(v);
627-
}
628-
else if (adios2sys::SystemTools::FileIsDirectory(name + ".tier0"))
629-
{
630-
engineTypeLC = "mhs";
631-
}
632637
else
633638
{
634-
if (helper::EndsWith(name, ".bp", false))
635-
{
636-
engineTypeLC = "bp3";
637-
}
638-
else
639+
// Rank 0 picks the engine from the on-disk dataset; an empty
640+
// result means nothing exists at the location.
641+
std::vector<char> selected;
642+
if (comm.Rank() == 0)
639643
{
640-
/* We need to figure out the type of file
641-
* from the file itself
642-
*/
643-
if (helper::IsHDF5File(name, *this, comm, m_TransportsParameters))
644+
std::string sel;
645+
if (adios2sys::SystemTools::PathExists(name) ||
646+
adios2sys::SystemTools::PathExists(name + ".tier0"))
644647
{
645-
engineTypeLC = "hdf5";
648+
if (adios2sys::SystemTools::FileIsDirectory(name))
649+
{
650+
sel = std::string("bp") + helper::BPVersionLocal(name);
651+
}
652+
else if (adios2sys::SystemTools::FileIsDirectory(name + ".tier0"))
653+
{
654+
sel = "mhs";
655+
}
656+
else if (helper::EndsWith(name, ".bp", false))
657+
{
658+
sel = "bp3";
659+
}
660+
else
661+
{
662+
sel = helper::IsHDF5FileLocal(name, *this, comm, m_TransportsParameters)
663+
? "hdf5"
664+
: "bp3";
665+
}
646666
}
647-
else
667+
selected.assign(sel.begin(), sel.end());
668+
}
669+
comm.BroadcastVector(selected);
670+
if (selected.empty())
671+
{
672+
// Built identically on every rank from 'name', so all ranks
673+
// throw together without broadcasting the message.
674+
std::string err = "Cannot determine the engine for reading \"" + name +
675+
"\": nothing exists at that location.";
676+
if (mode_to_use == Mode::Read)
648677
{
649-
engineTypeLC = "bp3";
678+
err += " If it is a stream that has not been created yet, select "
679+
"the engine explicitly with IO::SetEngine().";
650680
}
681+
helper::Throw<std::runtime_error>("Core", "IO", "Open", err);
651682
}
683+
engineTypeLC.assign(selected.begin(), selected.end());
652684
}
653685
}
654686
else
@@ -658,25 +690,6 @@ Engine &IO::Open(const std::string &name, const Mode mode, helper::Comm comm, co
658690
}
659691
}
660692

661-
/* Note: Mismatch between BP4/BP5 writer and FileStream reader is not
662-
handled if writer has not created the directory yet, when FileStream
663-
falls back to default (BP4) */
664-
if (engineTypeLC == "filestream")
665-
{
666-
if (helper::EndsWith(name, ".ats", false))
667-
{
668-
engineTypeLC = "timeseries";
669-
}
670-
else
671-
{
672-
char v = helper::BPVersion(name, comm, m_TransportsParameters);
673-
engineTypeLC = "bp";
674-
engineTypeLC.push_back(v);
675-
}
676-
// std::cout << "Engine " << engineTypeLC << " selected for FileStream"
677-
// << std::endl;
678-
}
679-
680693
// For the inline engine, there must be exactly 1 reader, and exactly 1
681694
// writer.
682695
if (engineTypeLC == "inline")

source/adios2/helper/adiosSystem.cpp

Lines changed: 35 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -124,62 +124,55 @@ int ExceptionToError(const std::string &function)
124124
}
125125
}
126126

127-
bool IsHDF5File(const std::string &name, core::IO &io, helper::Comm &comm,
128-
const std::vector<Params> &transportsParameters) noexcept
127+
bool IsHDF5FileLocal(const std::string &name, core::IO &io, helper::Comm &comm,
128+
const std::vector<Params> &transportsParameters) noexcept
129129
{
130130
bool isHDF5 = false;
131-
if (!comm.Rank())
131+
try
132132
{
133-
try
133+
transportman::TransportMan tm(io, comm);
134+
if (transportsParameters.empty())
134135
{
135-
transportman::TransportMan tm(io, comm);
136-
if (transportsParameters.empty())
137-
{
138-
std::vector<Params> defaultTransportParameters(1);
139-
defaultTransportParameters[0]["transport"] = "File";
140-
tm.OpenFiles({name}, adios2::Mode::Read, defaultTransportParameters, false);
141-
}
142-
else
143-
{
144-
tm.OpenFiles({name}, adios2::Mode::Read, transportsParameters, false);
145-
}
146-
const unsigned char HDF5Header[8] = {137, 72, 68, 70, 13, 10, 26, 10};
147-
if (tm.GetFileSize(0) >= 8)
148-
{
149-
char header[8];
150-
tm.ReadFile(header, 8, 0);
151-
tm.CloseFiles();
152-
isHDF5 = !std::memcmp(header, HDF5Header, 8);
153-
}
136+
std::vector<Params> defaultTransportParameters(1);
137+
defaultTransportParameters[0]["transport"] = "File";
138+
tm.OpenFiles({name}, adios2::Mode::Read, defaultTransportParameters, false);
154139
}
155-
catch (std::ios_base::failure &)
140+
else
156141
{
157-
isHDF5 = false;
142+
tm.OpenFiles({name}, adios2::Mode::Read, transportsParameters, false);
143+
}
144+
const unsigned char HDF5Header[8] = {137, 72, 68, 70, 13, 10, 26, 10};
145+
if (tm.GetFileSize(0) >= 8)
146+
{
147+
char header[8];
148+
tm.ReadFile(header, 8, 0);
149+
tm.CloseFiles();
150+
isHDF5 = !std::memcmp(header, HDF5Header, 8);
158151
}
159152
}
160-
size_t flag = (isHDF5 ? 1 : 0);
161-
flag = comm.BroadcastValue(flag);
162-
return (flag == 1);
153+
catch (std::ios_base::failure &)
154+
{
155+
isHDF5 = false;
156+
}
157+
return isHDF5;
163158
}
164159

165-
char BPVersion(const std::string &name, helper::Comm &comm,
166-
const std::vector<Params> &transportsParameters) noexcept
160+
bool IsHDF5File(const std::string &name, core::IO &io, helper::Comm &comm,
161+
const std::vector<Params> &transportsParameters) noexcept
167162
{
168-
char version = '4'; // default result
163+
size_t flag = 0;
169164
if (!comm.Rank())
170165
{
171-
std::string mmdFileName = name + PathSeparator + "mmd.0";
172-
if (adios2sys::SystemTools::PathExists(mmdFileName))
173-
{
174-
version = '5';
175-
}
176-
else
177-
{
178-
version = '4';
179-
}
166+
flag = IsHDF5FileLocal(name, io, comm, transportsParameters) ? 1 : 0;
180167
}
181-
version = comm.BroadcastValue(version);
182-
return version;
168+
flag = comm.BroadcastValue(flag);
169+
return (flag == 1);
170+
}
171+
172+
char BPVersionLocal(const std::string &name) noexcept
173+
{
174+
const std::string mmdFileName = name + PathSeparator + "mmd.0";
175+
return adios2sys::SystemTools::PathExists(mmdFileName) ? '5' : '4';
183176
}
184177

185178
unsigned int NumHardwareThreadsPerNode() { return std::thread::hardware_concurrency(); }

source/adios2/helper/adiosSystem.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,11 @@ int ExceptionToError(const std::string &function);
7373

7474
bool IsHDF5File(const std::string &name, core::IO &io, helper::Comm &comm,
7575
const std::vector<Params> &transportsParameters) noexcept;
76-
char BPVersion(const std::string &name, helper::Comm &comm,
77-
const std::vector<Params> &transportsParameters) noexcept;
76+
/** Rank-local IsHDF5File: no internal broadcast; call from a single rank. */
77+
bool IsHDF5FileLocal(const std::string &name, core::IO &io, helper::Comm &comm,
78+
const std::vector<Params> &transportsParameters) noexcept;
79+
/** Rank-local BP version sniff (no broadcast): '5' if an mmd.0 is present, else '4'. */
80+
char BPVersionLocal(const std::string &name) noexcept;
7881

7982
/** Return the number of available hardware threads on the node.
8083
* It might return 0 if the detection does not work

testing/adios2/interface/TestADIOSInterface.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,18 @@ TEST_F(ADIOS2_CXX_API_IO, EngineDefault)
169169
engine.Close();
170170
}
171171

172+
// Auto-selection (no SetEngine) on a missing path must throw on every rank, not
173+
// just rank 0, which otherwise hung the others (issue #5098).
174+
TEST_F(ADIOS2_CXX_API_IO, EngineSelectionMissingFileThrows)
175+
{
176+
EXPECT_THROW(m_Io.Open("ADIOSEngineSelectionMissingFile.bp", adios2::Mode::Read),
177+
std::exception);
178+
#if ADIOS2_USE_MPI
179+
// Returns only if every rank threw rather than hanging in Open.
180+
MPI_Barrier(MPI_COMM_WORLD);
181+
#endif
182+
}
183+
172184
template <class T>
173185
struct MyData
174186
{

0 commit comments

Comments
 (0)