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

Adding merging type (DAQ) and changing defaults #14528

Merged
merged 6 commits into from May 22, 2016

Conversation

smorovic
Copy link
Contributor

  • Merge type parameter added to output JSON to reflect difference between streams created by different output modules (different file format). This will be used to display streams differently in monitoring. Optionally, a PSet name can be specified and, if found in the configuration, will be parsed for per-stream overrides of this parameter (can be used e.g. if more specific differentiation of DAT streams is required for monitoring or merging).
  • by default activate empty-LS mode and disable micro-merging in CMSSW. This has been already configured as such in production HLT menus for some time.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @smorovic (Srecko Morovic) for CMSSW_8_0_X.

It involves the following packages:

DQMServices/Components
EventFilter/Utilities
HLTrigger/JSONMonitoring

@perrotta, @cmsbuild, @cvuosalo, @fwyzard, @emeschi, @dmitrijus, @Martin-Grunewald, @deguio, @slava77, @mommsen, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @barvic this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@mommsen
Copy link
Contributor

mommsen commented May 17, 2016

Hi Srecko,

I would propose to define the default values for the different stream types in EvFDaqDirector::getStreamMergeType instead of returning an empty string and have the default (string) values scattered over different places.

Remi

@dmitrijus
Copy link
Contributor

dmitrijus commented May 17, 2016

Hi Srecko,

I am trying to deprecate DQMFileSaver in favour of DQMFileSaverPB. So any changes done to former should go to the latter too:
https://github.com/cms-sw/cmssw/blob/CMSSW_8_1_X/DQMServices/FileIO/plugins/DQMFileSaverPB.cc#L99

Also, DQMFileSaver::fillJson is used here:
https://github.com/cms-sw/cmssw/blob/CMSSW_8_1_X/DQMServices/StreamerIO/plugins/JsonWritingTimeoutPoolOutputModule.cc#L42

(Used by Event Display stream written by the DQM)

Dmitrijus

@cmsbuild
Copy link
Contributor

Pull request #14528 was updated. @perrotta, @cmsbuild, @cvuosalo, @fwyzard, @emeschi, @dmitrijus, @Martin-Grunewald, @deguio, @slava77, @mommsen, @vanbesien, @davidlange6 can you please check and sign again.

@smorovic
Copy link
Contributor Author

Hi Remi,
I switched to using enums, with string values defined in static const vector in EvFDaqDirector. Modules are providing default enums which they want to use in case internal map doesn't contain their stream type.

Dmitrijus,
I'll add it to your new module.
Good point about JsonWritingTimeoutPoolOutputModule.cc. This module is not in HLT and doesn't use EvFDaqDirector. So I'll just put hardcoded "ROOT" there.

@cmsbuild
Copy link
Contributor

Pull request #14528 was updated. @perrotta, @cmsbuild, @cvuosalo, @fwyzard, @emeschi, @dmitrijus, @Martin-Grunewald, @deguio, @slava77, @mommsen, @vanbesien, @davidlange6 can you please check and sign again.

@mommsen
Copy link
Contributor

mommsen commented May 17, 2016

+1
Thanks @smorovic for implementing the change.

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@dmitrijus
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented May 17, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/13018/console

@mommsen
Copy link
Contributor

mommsen commented May 17, 2016

+1

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@Martin-Grunewald
Copy link
Contributor

+1
PR for 81X?

@smorovic
Copy link
Contributor Author

Created PR for 81X (#14543)

@slava77
Copy link
Contributor

slava77 commented May 18, 2016

+1

for #14528 02ebd55

  • offline reco is not affected

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@dmitrijus
Copy link
Contributor

dmitrijus commented May 18, 2016

Hi @smorovic,

do we need to apply this update to our (DQM) cluster, in order not to break the EventDisplay stream?
If so, when? I can imagine applying it too soon might break things too - these are positionals in a json array.

Dmitrijus

@smorovic
Copy link
Contributor Author

Hi @dmitrijus, as we already discussed in private mail thread, this doesn't need to be synchronized (merger will handle both JSON formats), so DQM can apply it independently of HLT.

@davidlange6 davidlange6 merged commit a5406f7 into cms-sw:CMSSW_8_0_X May 22, 2016
@smorovic smorovic deleted the mergeType-80X branch February 13, 2019 11: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

7 participants