Centralize schema-safe writable ROOT I/O (preserve StreamerInfo on ROOT ops) - #567
Open
cmargalejo wants to merge 5 commits into
Open
Centralize schema-safe writable ROOT I/O (preserve StreamerInfo on ROOT ops)#567cmargalejo wants to merge 5 commits into
StreamerInfo on ROOT ops)#567cmargalejo wants to merge 5 commits into
Conversation
Member
Author
|
@AlvaroEzq, @lobis, @hgmaluenda, @juanangp , small ping on this PR. We want to get a new stable REST release ready, and this is one of the PRs still waiting for review. Could you have a look when you get a chance? |
juanangp
approved these changes
Jun 15, 2026
Introduce a move-only TRestRootFileHandle that inventories the exact on-disk StreamerInfo tuples and schema rules before a writable open. UPDATE now starts in READ mode, prepares loaded and emulated classes, preflights every historical entry, verifies that the file identity is unchanged, and only then reopens the same TFile handle for mutation. Make the policy explicit: remote files remain valid read sources but cannot be writable destinations, local file URLs are supported, and close failures are observable. Exact class/version/checksum entries are marked instead of broadly mutating unrelated StreamerInfo state. Add a transactional ROOT merge helper that validates recursive key/class manifests, TTree entry totals, and schema inventories before and after atomic installation. Existing targets are protected by rollback backups, input files are removed only after complete validation, and cleanup or rollback failures are surfaced to callers.
Replace ad-hoc writable TFile opens in framework macros, data-set helpers, runs, process runners, and threads with TRestRootFileHandle. Keep transient owning handles beside legacy raw aliases where ROOT reflection requires them, and make TRestRun non-copyable so its file state cannot be shallow-copied. Use the transactional merge path for TRestRun outputs, including the create-if-absent case, and leave source files intact whenever validation, replacement, backup cleanup, or input cleanup fails. Analysis-chain attachment is restored to read-only operation. Write split process metadata to an explicit destination rather than relying on gDirectory, and make input mode handling explicit: READ/OPEN and UPDATE are accepted while destructive or unsupported modes fail clearly.
Build independent version-1 and version-2 ROOT dictionaries in which fSamples keeps the same member name while changing from vector<float> to vector<double>. Deliberately provide no pragma for that member: the version-2 reader first proves ROOT's automatic same-name collection conversion before REST mutates the file, and repeats the value checks after unloaded and loaded update paths. Exercise embedded schema-rule preservation independently with a renamed scalar, mapping the legacy fLegacyCode member to fRenamedCode through a pragma. Assert that the on-disk rule inventory contains this renamed-field rule and no rule for fSamples, verify the rule-based value conversion before and after updates, and continue checking the original payload with the matching version-1 dictionary. Preserve both exact historical payload StreamerInfo tuples while covering preflight byte preservation, remote mutation policy, file URL and move-only lifecycles, unsupported run modes, and explicit split-metadata destinations. Compile-time assertions protect the intended ownership semantics. Exercise transactional merging with recursive target-only and input-only keys, aggregate TTree entry checks, incompatible same-path classes, missing sources, and a forced post-install validation failure. The failure tests require byte-identical rollback and retention of every source file.
Add a developer guide for TRestRootFileHandle and PrepareBorrowedUpdate, including the read-first schema preflight, preservation of historical StreamerInfo and embedded rules, local-only mutation policy, move-only ownership, and explicit Close/error handling. Document transactional merge and replacement behavior, how existingTarget participates, when inputs are removed, and how callers must interpret cleanup and rollback failures. Bound the guarantee to the implemented platform filesystem operations rather than promising crash durability or atomicity on every mounted filesystem. Explain why TRestRun is intentionally non-copyable and give practical migration paths through references, pointers, unique ownership, or reconstruction. Link the guide from CONTRIBUTING.md so it is discoverable from the repository's developer documentation entry point.
Vindaar
force-pushed
the
cris_preserve_streamerinfos
branch
from
August 19, 2026 13:21
520b1fa to
e0e156d
Compare
for more information, see https://pre-commit.ci
StreamerInfo on ROOT ops)
Vindaar
approved these changes
Aug 19, 2026
Vindaar
left a comment
Member
There was a problem hiding this comment.
The addition of the new changes should make it so that REST files should remain stable and readable in the future even under further schema changes. That's a big win for us, because there's nothing worse than breaking backwards compatibility for old data files. Especially given the context of the likely changes still to come for BabyIAXO once we get closer to actual data taking. At some point development will speed up and we are going to be happy this is done now and not later.
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
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.
Fixes #568
UPDATE 2026-08-19:
Also updating this PR significantly. The idea was essentially correct, but it didn't go far enough. The issue is that with the PR in the original form there would be many edge cases where we could have lost the
StreamerInfoin the future. Either through existing code paths or more importantly due to the way the developers of REST need to use the ROOT I/O in order to not lose that information.TRestRootFileHandle.StreamerInfoand schema rules, including classes whose dictionaries are not currently loaded.TFilecall sites are migrated to this checked path.TRestRunbecomes non-copyable because it now owns file handles and contains non-owning aliases into their contents.vector<float> → vector<double>evolution, explicit renamed-member rules, update failures, merges, rollback, and remote paths.So #566 repairs the specific damaged legacy detector-signal files; #567 prevents normal REST updates and merges from discarding the historical schema information needed to read old data safely.
Important note for developers: In the future it is mandatory that all ROOT I/O happens through the new safe interop. See the documentation about this here:
https://github.com/rest-for-physics/framework/blob/07c720c6cd89f594c2047a3bbdff609c2e7f3c89/doc/developer/Safe%20writable%20ROOT%20IO.md
In principle this should make our lives easier.
The addition of the new I/O interface and updating of all existing ROOT file I/O code is the reason the PR exploded in size now.
Original PR message
The problem
When a ROOT file is closed,
TFile::WriteStreamerInfo()replaces the file'sStreamerInfo record, keeping only the infos of the classes streamed during that
session.
TRestRun::MergeToOutputFilemerges the threads' output files (whichpreserves the infos), but then reopens the merged file in UPDATE mode to write
the metadata: closing that session wipes the StreamerInfos of all event classes,
keeping only the metadata classes.
As a consequence, no restManager output file contains event-class
StreamerInfos, which silently breaks ROOT schema evolution whenever an event
class definition changes. This is what made files written before the
TRestDetectorSignalvector<Float_t>→vector<Double_t>change unreadable(rest-for-physics/detectorlib#125): without the on-disk layout description,
ROOT misreads the float payload as doubles and allocates GBs of garbage.
(restG4 files are unaffected: they are written in a single session.)
The fix
New
TRestTools::PreserveStreamerInfos(TFile*): called right after opening afile in UPDATE mode, it re-tags every StreamerInfo already stored in the file
(same marking as the deprecated
TStreamerInfo::TagFile) so thatTFile::WriteStreamerInfowrites them out again on close. Called at the UPDATEsessions that write into REST data files:
TRestRun::MergeToOutputFile(the main restManager output path)TRestRun::UpdateOutputFileTRestProcessRunnersplit-file metadata update of the main fileTRestDataSet::ExportVerification
Merging two files containing
TRestDetectorSignalEventtrees viaTRestRun::MergeToOutputFile: without the fix the merged file keeps onlyTRestRun/TRestMetadatainfos; with it,TRestDetectorSignalEvent,TRestEventandTRestDetectorSignalsurvive the metadata session and thefile reads back correctly.
Note: this protects files written from now on. Existing files have already
lost their StreamerInfos; for the detector signal case the data is recoverable
with the tool in #566.