MEtoEDMConverter does not need synchronizing on Run and Lumi boundaries

[Dr15Jones]

Using a RunCache and LuminosityBlockCache signals to the framework
the module will not require synchronization on those boundaries.

[cmsbuild]

The code-checks are being triggered in jenkins.

[andrius-k]

please test

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25105/7099

[cmsbuild]

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

[cmsbuild]

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

It involves the following packages:

DQMServices/Components

@kmaeshima, @cmsbuild, @andrius-k, @jfernan2, @schneiml can you please review it and eventually sign? Thanks.
@barvic 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

[cmsbuild]

+1
Tested at: e5ce748
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25105/31446/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
159c739
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25105/31446/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25105/31446/git-merge-result

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

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

[schneiml]

@Dr15Jones while I believe your reasoning is right, we should check that the output produced is still correct. I remember this is a fairly fragile mechanism and it might rely on undefined behavior in the framework. I'll try to find the wf that tests this module (IIRC there is one) and check.

[Dr15Jones]

@schneiml I completely agree that this needs proper testing before being used for production work. Again, I encourage the DQM L2s to work with @fabiocos @smuzaffar and myself to create automated testing systems for the DQM modules. This is particular important as changes to the DQM modules will be needed to accomodate the greater concurrency needed during Run 3 and Run 4.

[schneiml]

@Dr15Jones I agree on better testing, but in this case the situation is a bit different: the code actually runs, but currently we don't really look at the infrastructure that compares DQM output (esp. not for these "non-standard" configurations).

And yes, I have some plans for the long shutdown, and better testing infrastructure is a big precondition for that. Actually, some test coverage tool would be cool...

[Dr15Jones]

@schneiml wrote: I remember this is a fairly fragile mechanism and it might rely on undefined behavior in the framework.

This change does not change the timing of when the framework calls the member functions of this module, i.e. it is still called on globalEndLuminosityBlock. Therefore it should be no better or worse if the code is relying on undefiled behavior.

[schneiml]

@Dr15Jones good to hear.

I wanted to repeat the test from back in #22281 (basically run 1001/1004 and check if all the root files have properly filled histograms), but didn't get to it yet.

[Dr15Jones]

@schneiml there is no rush from my side. Please run the test when it is convenient.

[Dr15Jones]

I wanted to repeat the test from back in #22281 (basically run 1001/1004 and check if all the root files have properly filled histograms), but didn't get to it yet.

@schneiml any timetable on when this will happen?

[schneiml]

Hi Chris, seems I forgot to report here that I did this test and nothing interesting happened (which is good). So no concerns from my side.

[Dr15Jones]

@schneiml Is there anything you need done before you sign this pull request?

[andrius-k]

+1

[cmsbuild]

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)

[Dr15Jones]

@fabiocos what is your plan for integrating this?

[Dr15Jones]

@fabiocos what is your plan for integrating this?

[fabiocos]

+1

@Dr15Jones sorry, this was left behind in the flood of recent updates, as @schneiml tests do not show any issue, let's see in the IB
@schneiml could you please clarify which tests were done? Are they already available in the standard panel of test workflows? Probably not I guess

[schneiml]

@fabiocos The tests run in the runTheMatrix IB tests, but the files (AlCa related output files) are not part of any comparison. We might provide a solution for that soonish.