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.

Sign up for GitHub

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

Jump to bottom

Automatically switch DQMStore to threaded mode #3411

Merged
merged 1 commit into from Apr 22, 2014
Merged

Automatically switch DQMStore to threaded mode #3411

merged 1 commit into from Apr 22, 2014

Conversation

Dr15Jones
Copy link
Contributor

Have the DQMStore automatically switch to threaded mode when the
configuration for the job has more than one stream.

@Dr15Jones
Automatically switch DQMStore to threaded mode
197ddb3 
Have the DQMStore automatically switch to threaded mode when the
configuration for the job has more than one stream.
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Dr15Jones (Chris Jones) for CMSSW_7_1_X.

Automatically switch DQMStore to threaded mode

It involves the following packages:

DQMServices/Core

@ojeda, @danduggan, @rovere, @cmsbuild, @nclopezo, @deguio, @Degano can you please review it and eventually sign? Thanks.
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.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@Dr15Jones
Copy link
Contributor Author

@ktf This really needs to be in before we merge in the _THREADED branch.

@danduggan
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes or unless it breaks tests. @nclopezo, @ktf can you please take care of it?

@rovere
Copy link
Contributor

rovere commented Apr 21, 2014

Ciao @Dr15Jones, all,
thanks for this PR. I still have few thoughts about it that I write down.

  1. A similar PR should be done for DQMIO, in particular for the DQMRootOutputModule, since also this piece of code is sensitive to multithread or not while retrieving the MEs to be persisted from the DQMStore. It will not be important, IIRC, until after we have event-based parallelism, w/ events coming form different Runs/LS processed at the same time.
  2. The same argument is true for DQMFileSaver. I can offer to work on this (and the previous one) tomorrow, since Fede is on (a well deserved) vacation.
  3. I'm still not totally convinced that mixing old modules w/ the multithread behavior of the DQMStore will produces meaningful results. Again, probably this will not be important if we process events coming from the same run... Yet, I do think that a correct behavior is guaranteed only by DQMEDAnalyzers+DQMStore thread-friendly. All other combination could work but have not been certified, at least not by me while doing all the integration tests. The goals was of course to have the flag set when all modules have been migrated.

Good in any case that the flag does what it was supposed to do: I had a sleepless night last night ;)

Ciao,
Marco.

@cmsbuild
Copy link
Contributor

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3411/955/summary.html

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

nclopezo added a commit that referenced this pull request Apr 22, 2014
@nclopezo
Merge pull request #3411 from Dr15Jones/autoSetDQMStoreToThreadedMode
ee18920 
DQMServices/Core -- Automatically switch DQMStore to threaded mode
@nclopezo nclopezo merged commit ee18920 into cms-sw:CMSSW_7_1_X Apr 22, 2014
@cmsbuild
Copy link
Contributor

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

@Dr15Jones
Copy link
Contributor Author

@rovere I believe mixing legacy and DQMEDAnalyzer in the same job should be just fine assuming all DQMEDAnalyzer modules only ever 'talk to' the DQMStore in their bookHistograms function.
In the future case where the framework supports concurrent Runs/Lumis being processed, if there is a legacy module in the job then they framework will automatically only allow 1 Run/Lumi to be processed at a time.

@Dr15Jones Dr15Jones deleted the autoSetDQMStoreToThreadedMode branch April 22, 2014 13:56
rovere added a commit to rovere/cmssw that referenced this pull request Jan 13, 2016
@rovere
Fix DQM standalone compilation
960f7c3 
DQM core code needs to be compiled outside of the CMSSW FWK into the
DQMGUI code. This PR fixes few bugs that have been introduced in the
past (commit
cms-sw@197ddb3,
PR cms-sw#3411). All the include files
that are CMSSW related should never be added directly in DQMStore.cc,
but to Standalone.h protected by the WITHOUT_CMS_FRAMEWORK condition. A
required edm::service namespace was missing, and this bug allowed the
original compilation of the code that would have otherwise correctly
failed to compile. This indicates that a better way of testing the
standalong compilation should be found, but that's not extremely urgent.
@rovere rovere mentioned this pull request Jan 13, 2016
Merged
cmsbuild pushed a commit that referenced this pull request Jan 16, 2016
@rovere @cmsbuild
Fix DQM standalone compilation
2061da1 
DQM core code needs to be compiled outside of the CMSSW FWK into the
DQMGUI code. This PR fixes few bugs that have been introduced in the
past (commit
197ddb3,
PR #3411). All the include files
that are CMSSW related should never be added directly in DQMStore.cc,
but to Standalone.h protected by the WITHOUT_CMS_FRAMEWORK condition. A
required edm::service namespace was missing, and this bug allowed the
original compilation of the code that would have otherwise correctly
failed to compile. This indicates that a better way of testing the
standalong compilation should be found, but that's not extremely urgent.
(cherry picked from commit 960f7c3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
comparison-pending dqm-approved fully-signed tests-approved
Projects
None yet
Milestone
CMSSW_7_1_0_pre7
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Dr15Jones @cmsbuild @danduggan @rovere @nclopezo