-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle event meta data changes in streamer files #44892
Conversation
please test |
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44892/40158
|
A new Pull Request was created by @Dr15Jones for master. It involves the following packages:
@saumyaphor4252, @perrotta, @nothingface0, @smuzaffar, @tjavaid, @syuvivida, @antoniovagnerini, @makortel, @smorovic, @consuegs, @rvenditti, @Dr15Jones, @emeschi can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-79e450/39218/summary.html Comparison SummarySummary:
|
Part of addressing #44643 |
CalibCalorimetry/EcalLaserSorting/src/WatcherStreamFileReader.cc
Outdated
Show resolved
Hide resolved
The event meta-data (e.g. BranchLists) is now added as an EventMsg before the following EventMsg for which it applies. This added EventMsg contains no data products. When seen, the added EventMsg is handled as an artificial file boundary in order to cause the needed stream synchronization. This change is needed to handle how HLT merges parts of streamer files which might have different meta-data.
- event meta data is cached at begin job or when new file is opened - refactored StreamerOutputModuleCommon to be two classes so buffer could be handled separately for the caching - added many unit tests to show GlobalEvFOutputModule properly writes the expected events and the data products are correct
e3d9efe
to
d9b3643
Compare
REMINDER @rappoccio, @sextonkennedy, @antoniovilela: This PR was tested with #44991, please check if they should be merged together |
ping @cms-sw/dqm-l2 |
+1 |
+1 |
is this still meant to be on hold? |
unhold |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will be automatically merged. |
@Dr15Jones , this is now available in today's 11h IB. There is a unit test failing with [a], could this PR be the cause of it? Note that IB is a patch build, may be we need a full build to avoid it?
|
It is.
Full build won't help. Based on the logs the test seems to be reading streamer files (brittle), so either the streamer files need to be redone (through streamer -> root -> streamer cycle), or the files could be converted to root format if the streamer aspect is not super important for that test (or the test would be disabled). I'd suggest to follow up with the test authors in a new issue. |
I opened an issue in #45101 |
} | ||
|
||
void GlobalEvFOutputModule::acquire(edm::StreamID id, | ||
edm::EventForOutput const& e, | ||
edm::WaitingTaskWithArenaHolder iHolder) const { | ||
edm::Handle<edm::TriggerResults> const& triggerResults = getTriggerResults(trToken_, e); | ||
|
||
auto streamerCommon = streamCache(id); | ||
std::cout << " writing Event " << moduleDescription().moduleLabel() << std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this cout
call meant to stay ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done for master
(14_1_X) in #45125.
PR description:
Moved storage of meta data needed for events into the first EventMessage sent before the subsequent events which need that meta data.
This fixed a problem seen online where different HLT nodes were generating different event meta data for the same luminosity block but online the event data for one node is used.
PR validation:
All unit tests associated with streamer files pass.