Skip to content
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

multithreading fix for 74x: EcalRingCalibrationTools #11348

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Sep 18, 2015

Implement a minimal set of changes to EcalRingCalibrationTools to make
its initialisation and usage thead-safe.
In particular, this should make HLTEcalPhiSymFilter thread-safe again.

Note that EcalRingCalibrationTools is still non-compliant with the CMS
coding rules, and should be rewritten either as an ESProducer, or as a
non-static data member of an EDProducer.

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 18, 2015

please test

@cmsbuild cmsbuild added this to the Next CMSSW_7_4_X milestone Sep 18, 2015
@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 18, 2015

tracked at #11317

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @fwyzard (Andrea Bocci) for CMSSW_7_4_X.

make EcalRingCalibrationTools thread safe

It involves the following packages:

Calibration/Tools
HLTrigger/special

@perrotta, @cmsbuild, @diguida, @cerminar, @Martin-Grunewald, @fwyzard, @mmusich can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 21, 2015

backport #11093 / #11094 to 74x

@fwyzard fwyzard changed the title make EcalRingCalibrationTools thread safe make EcalRingCalibrationTools thread safe (74x) Sep 21, 2015
Implement a minimal set of changes to EcalRingCalibrationTools to make
its initialisation and usage thead-safe.
In particular, this should make HLTEcalPhiSymFilter thread-safe again.

Note that EcalRingCalibrationTools is still non-compliant with the CMS
coding rules, and should be rewritten either as an ESProducer, or as a
non-static data member of an EDProducer.
@fwyzard fwyzard force-pushed the multithreading_74x_EcalRingCalibrationTools branch from 4d6b4b1 to fa48c54 Compare September 23, 2015 19:08
@fwyzard fwyzard changed the title make EcalRingCalibrationTools thread safe (74x) multithreading fix for 74x: EcalRingCalibrationTools Sep 23, 2015
@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 23, 2015

rebased to merge on top of both CMSSW_7_4_X and CMSSW_7_4_12_patch4

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 23, 2015

please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

Pull request #11348 was updated. @perrotta, @cmsbuild, @diguida, @cerminar, @Martin-Grunewald, @franzoni, @fwyzard, @mmusich can you please check and sign again.

@cmsbuild
Copy link
Contributor

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 23, 2015

+1

@cmsbuild
Copy link
Contributor

@davidlange6
Copy link
Contributor

+1

davidlange6 added a commit that referenced this pull request Oct 10, 2015
…ibrationTools

multithreading fix for 74x: EcalRingCalibrationTools
@davidlange6 davidlange6 merged commit 323191c into cms-sw:CMSSW_7_4_X Oct 10, 2015
@fwyzard fwyzard deleted the multithreading_74x_EcalRingCalibrationTools branch October 10, 2015 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants