Refactor to drop TransportMan where we can (not bp3/4)#5048
Merged
Conversation
pnorbert
reviewed
May 27, 2026
Lifts the body of TransportMan::OpenFileTransport into a new transport::OpenFile (and OpenFileChained) free function. The TransportMan method becomes a one-line forwarder so existing callers are unaffected. First step toward dropping TransportMan from non-frozen engines.
ReadRemoteFile constructed a TransportMan only to call OpenFileTransport on it once and discard. Use the free function instead. Also drops an unused local.
FilePool and SharedTarFDCache now decouple from TransportMan via a TransportOpener typedef (std::function<shared_ptr<Transport>(name, params)>). Callers pass a lambda that constructs transports however they like. BP5Reader's FilePool construction sites switch to lambdas wrapping transport::OpenFile. m_TransportFactory becomes unused; deletion in a follow-up commit.
With FilePool taking a TransportOpener lambda, the TransportMan member is no longer referenced. Remove the field, its constructor init, and the TransportMan.h include.
Adds transport::MkDirsBarrier alongside OpenFile/OpenFileChained. TransportMan::MkDirsBarrier forwards to it so frozen engines (BP3/BP4/DAOS) are unaffected. BP5Writer's factory-hat m_TransportFactory is now unused: replace its three OpenFileTransport calls with transport::OpenFile, the MkDirsBarrier call with the free function, and delete the member.
io.AddTransport() can be called multiple times to attach parallel transports, but no caller (test, example, or committed config) actually does this for a BP5 engine. Throw at Init() so the historical multiplexer paths don't have to remain plausibly multi-transport-safe.
AggTransportData now holds std::vector<std::shared_ptr<Transport>> m_DataSubstreams instead of a TransportMan. All data writes, opens, flushes, truncates, seeks, closes, and async-thread handoffs become explicit loops over the vector. The 'transportIndex' parameter to DoClose is now ignored: multi-transport configs are rejected at Init (see prior commit), so close always covers every substream. AsyncWriteInfo carries a pointer to the substream vector instead of a TransportMan* so async writer threads iterate without going through the manager.
Only frozen engines (BP3, BP4) and the not-yet-migrated DAOS engine still use TransportMan. Steer new code toward transport::OpenFile and bare shared_ptr<Transport> members.
Mirrors the BP5 migration (016286c + 69f0cce): DaosWriter and DaosReader hold std::shared_ptr<Transport> for the metadata files and open them via transport::OpenFile / transport::MkDirsBarrier. Init hard-errors when m_IO.m_TransportsParameters.size() > 1, matching the BP5 reject pattern. Reader side also drops dead code that survived the BP5 fork: ReadData never actually used its FileManager / maxOpenFiles parameters (DAOS data reads go straight to daos_array_read against m_DataArrayHandles), so the per-thread fileManagers vector, m_DataFileManager, m_ActiveFlagFileManager, and singleComm are all removed. The PerformGets thread lambda loses its now-unused FileManagerID and maxOpenFiles arguments. DestructorClose's UpdateActiveFlag(false) is now correctly guarded to rank 0 (the prior TransportMan version no-op'd on empty maps, so the missing guard was latent). The transportman::TransportMan::GetFilesBaseNames static helper call in InitTransports is left as-is; same pattern as BP5Writer post-migration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Test file was missed in 7da002c (FilePool API change). Replaces the TransportMan factory with a lambda wrapping transport::OpenFile in each test, matching the BP5Reader caller pattern.
AggTransportData's m_DataSubstreams (vector), m_SubStreamNames (vector), and m_DrainSubStreamNames (vector) become singular shared_ptr<Transport> and string members. The vectors were size-1 by construction (GetBPSubStreamNames is called with a single base name, and the user multi-transport feature is rejected at Init). Drops the for-each loops over m_DataSubstreams in the write path and across the async writer, where the size-1 invariant made the loop vestigial. AsyncWriteInfo carries a shared_ptr<Transport>* instead of a vector*. CloseFiles and FlushProfiler iterations now null-check m_DataSubstream: an AggTransportData entry may exist with no open transport when the rank hasn't written through that aggregator.
eisenhauer
force-pushed
the
drop-transportman-bp5
branch
from
140e51d to
f99b52e
Compare
pnorbert
previously approved these changes
May 27, 2026
WriteData calls Flush/FinalizeSegment on aggData.m_DataSubstream unconditionally after the per-mode write dispatch. In TwoLevelShm-style modes only the aggregator rank holds an open transport; the original vector-based code silently no-op'd on non-aggregator ranks (empty vector). The post-singularization code crashed with a null deref — caught by the lammps contract test. Restores the no-op.
Member
Author
|
@pnorbert One more fix after eliminating the loop... |
pnorbert
approved these changes
May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.