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

Prepare RPCInverse ESProducers for concurrent IOVs #24511

Merged
merged 1 commit into from Sep 20, 2018

Conversation

wddgit
Copy link
Contributor

@wddgit wddgit commented Sep 11, 2018

Currently the Framework only allows one EventSetup IOV (Interval of Validity) to be processed at a time, but we are preparing to change this in the future and allow multiple concurrent IOVs. This PR modifies the ESProducers related to RPCs in preparation for this change.

Prior to this, the ESProducer held a pointer to a single produced object. After this PR it will use the ReusableObjectHolder class to manage multiple objects, one for each concurrent IOV. The objects needs to hold different content for different IOVs and the ESProducer needs to modify them during its produce call in a thread safe manner.

The dependsOn feature of the EventSetup has difficulty communicating with these different objects in the callbacks and produce function calls. The usage of the dependsOn feature is replaced by usage of the new ESProductHost class. It has a similar purpose, but works better when there are multiple IOVs because its interface allows direct communication via arguments between the produce methods and the lambda function that gets called when the dependent record changes.

Note, I could not find anything in CMSSW that actually uses these modules. (It is possible they are not needed at all.) The standard tests I tried do not run them, so to test I manually created a test configuration that runs them and stepped through in the debugger to see they were performing as I expected.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @wddgit (W. David Dagenhart) for master.

It involves the following packages:

CondTools/RPC

@ggovi, @cmsbuild can you please review it and eventually sign? Thanks.
@mmusich this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@wddgit
Copy link
Contributor Author

wddgit commented Sep 11, 2018

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 11, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30345/console Started: 2018/09/11 23:02

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24511/30345/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3147363
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3147165
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 133 log files, 14 edm output root files, 32 DQM output files

@ggovi
Copy link
Contributor

ggovi commented Sep 18, 2018

+1

@cmsbuild
Copy link
Contributor

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)

@fabiocos
Copy link
Contributor

@ggovi following the comment of @wddgit where is this code practically used? P5 operations?

@fabiocos
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit ec8f8f8 into cms-sw:master Sep 20, 2018
@wddgit wddgit deleted the rpcConcurrentIOVs branch January 14, 2019 15:33
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

4 participants