-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix LuminosityBlockSummaryCache & concurrent lumis #28362
Conversation
Fix a bug affecting LuminosityBlockSummaryCache when processing more than 1 lumi concurrently. Nothing in production runs with concurrent lumis yet so I am guessing the impact of this is small. Outside of Core tests, the only modules using this cache at this point in time are HLTriggerJSONMonitoring and L1TriggerJSONMonitoring.
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28362/12664
|
A new Pull Request was created by @wddgit (W. David Dagenhart) for master. It involves the following packages: FWCore/Framework @cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
+1 |
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 now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
Comparison is ready Comparison Summary:
|
please test the DQM changes look strange, likely unrelated to this PR, I wonder whether they are related to #28330 |
The tests are being triggered in jenkins. |
+1 the changes appear also in other PRs, so I consider ok to merge this one. Anyway the test will be useful |
merge |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
PR description:
Fix a bug affecting LuminosityBlockSummaryCache only when processing more than 1 lumi concurrently. Nothing in production runs with more than 1 concurrent lumis yet, so I am guessing the impact of this bug has been small. Outside of Core tests, the only modules in the CMSSW repository using this cache at this point in time are HLTriggerJSONMonitoring and L1TriggerJSONMonitoring.
This fixes the problem reported by Srecko Morovic on the Framework hyper news on 10/31/19.
PR validation:
Added a new Core unit test to verify correct behavior. This test would have detected the original problem reported.